[Devel] [PATCH 2/4] fs/fuse kio: add pending kio requests to kqueue
Pavel Butsykin
pbutsykin at virtuozzo.com
Wed May 15 14:09:29 MSK 2019
On 15.05.2019 13:26, Kirill Tkhai wrote:
> Hi, Pavel,
>
> On 15.05.2019 11:53, Pavel Butsykin wrote:
>> Pending kio requests don't fall into kqueue list and therefore not tracked, this
>> of course is a mistake. This patch fixes the mistake, making it possible to add
>> pending requests to di->kq inside pcs_fuse_prep_rw(). It's also very important
>> to be able to immediately interrupt pending kio requests and terminate it with
>> error in order to maintain synchronization with fuse_invalidate_files(). By this
>> reason pcs_fuse_prep_rw() will return -EIO in case FUSE_S_FAIL_IMMEDIATELY
>> status was set to ff_state.
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
>> ---
>> fs/fuse/kio/pcs/fuse_io.c | 5 +++
>> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 82 +++++++++++++++++++++++++-------------
>> 2 files changed, 59 insertions(+), 28 deletions(-)
>>
>> diff --git a/fs/fuse/kio/pcs/fuse_io.c b/fs/fuse/kio/pcs/fuse_io.c
>> index 219f4e3423af..ed5926eb5d4d 100644
>> --- a/fs/fuse/kio/pcs/fuse_io.c
>> +++ b/fs/fuse/kio/pcs/fuse_io.c
>> @@ -253,10 +253,15 @@ void pcs_fuse_prep_io(struct pcs_fuse_req *r, unsigned short type, off_t offset,
>> static void falloc_req_complete(struct pcs_int_request *ireq)
>> {
>> struct pcs_fuse_req * r = ireq->completion_data.priv;
>> + struct pcs_dentry_info *di = get_pcs_inode(r->req.io_inode);
>> struct pcs_fuse_cluster *pfc = cl_from_req(r);
>>
>> BUG_ON(ireq->type != PCS_IREQ_NOOP);
>>
>> + spin_lock(&di->kq_lock);
>> + list_del_init(&r->req.list);
>> + spin_unlock(&di->kq_lock);
>> +
>> DTRACE("do fuse_request_end req:%p op:%d err:%d\n", &r->req, r->req.in.h.opcode, r->req.out.h.error);
>> fuse_stat_account(pfc->fc, KFUSE_OP_FALLOCATE, ktime_sub(ktime_get(), ireq->ts));
>> inode_dio_end(r->req.io_inode);
>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> index efa2b4054215..e75eaf090d99 100644
>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> @@ -815,6 +815,19 @@ static void wait_shrink(struct pcs_fuse_req *r, struct pcs_dentry_info *di)
>> list_add_tail(&r->exec.ireq.list, &di->size.queue);
>> }
>>
>> +static bool kqueue_insert(struct pcs_dentry_info *di, struct fuse_file *ff,
>> + struct fuse_req *req)
>> +{
>> + spin_lock(&di->kq_lock);
>> + if (ff && test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state)) {
>> + spin_unlock(&di->kq_lock);
>> + return false;
>> + }
>> + list_add_tail(&req->list, &di->kq);
>> + spin_unlock(&di->kq_lock);
>> + return true;
>> +}
>> +
>> /*
>> * Check i size boundary and deffer request if necessary
>> * Ret code
>> @@ -822,7 +835,7 @@ static void wait_shrink(struct pcs_fuse_req *r, struct pcs_dentry_info *di)
>> * -1: should fail request
>> * 1: request placed to pended queue
>> */
>> -static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>> +static int pcs_fuse_prep_rw(struct pcs_fuse_req *r, struct fuse_file *ff)
>> {
>> struct fuse_inode *fi = get_fuse_inode(r->req.io_inode);
>> struct pcs_dentry_info *di = pcs_inode_from_fuse(fi);
>> @@ -832,8 +845,8 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>> /* Deffer all requests if shrink requested to prevent livelock */
>> if (di->size.op == PCS_SIZE_SHRINK) {
>> wait_shrink(r, di);
>> - spin_unlock(&di->lock);
>> - return 1;
>> + ret = 1;
>> + goto out;
>> }
>> if (r->req.in.h.opcode == FUSE_READ) {
>> size_t size;
>> @@ -843,8 +856,8 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>> if (in->offset + in->size > di->fileinfo.attr.size) {
>> if (in->offset >= di->fileinfo.attr.size) {
>> r->req.out.args[0].size = 0;
>> - spin_unlock(&di->lock);
>> - return -1;
>> + ret = -EPERM;
>> + goto out;
>> }
>> size = di->fileinfo.attr.size - in->offset;
>> }
>> @@ -853,6 +866,10 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>> struct fuse_write_in *in = &r->req.misc.write.in;
>>
>> if (in->offset + in->size > di->fileinfo.attr.size) {
>> + if (!kqueue_insert(di, ff, &r->req)) {
>> + ret = -EIO;
>> + goto out;
>> + }
>> wait_grow(r, di, in->offset + in->size);
>> ret = 1;
>> }
>> @@ -868,8 +885,8 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>> size = in->fm_length;
>> if (in->fm_start + size > di->fileinfo.attr.size) {
>> if (in->fm_start >= di->fileinfo.attr.size) {
>> - spin_unlock(&di->lock);
>> - return -1;
>> + ret = -EPERM;
>
> What is the reason, that EPERM is here?
>
>> + goto out;
>> }
>> size = di->fileinfo.attr.size - in->fm_start;
>> }
>> @@ -880,6 +897,10 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>> struct fuse_fallocate_in const *in = r->req.in.args[0].value;
>>
>> if (in->offset + in->length > di->fileinfo.attr.size) {
>> + if (!kqueue_insert(di, ff, &r->req)) {
>> + ret = -EIO;
>> + goto out;
>> + }
>> wait_grow(r, di, in->offset + in->length);
>> ret = 1;
>> }
>> @@ -892,14 +913,14 @@ static int pcs_fuse_prep_rw(struct pcs_fuse_req *r)
>> if (ret) {
>> pcs_fuse_prep_fallocate(r);
>> } else {
>> - spin_unlock(&di->lock);
>> - return -1;
>> + ret = -EPERM;
>> + goto out;
>> }
>> }
>> }
>> inode_dio_begin(r->req.io_inode);
>> +out:
>> spin_unlock(&di->lock);
>> -
>> return ret;
>> }
>>
>> @@ -924,12 +945,15 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>> switch (r->req.in.h.opcode) {
>> case FUSE_WRITE:
>> case FUSE_READ:
>> - ret = pcs_fuse_prep_rw(r);
>> - if (!ret)
>> + ret = pcs_fuse_prep_rw(r, ff);
>> + if (likely(!ret))
>> goto submit;
>> if (ret > 0)
>> - /* Pended, nothing to do. */
>> - return;
>> + return; /* Pended, nothing to do. */
>> + if (ret != -EPERM) {
>> + req->out.h.error = ret;
>> + goto error;
>>
>> + }
>> break;
>> case FUSE_FALLOCATE: {
>> struct fuse_fallocate_in *inarg = (void*) req->in.args[0].value;
>> @@ -958,12 +982,15 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>> inarg->length = di->fileinfo.attr.size - inarg->offset;
>> }
>>
>> - ret = pcs_fuse_prep_rw(r);
>> - if (!ret)
>> + ret = pcs_fuse_prep_rw(r, ff);
>> + if (likely(!ret))
>> goto submit;
>> if (ret > 0)
>> - /* Pended, nothing to do. */
>> - return;
>> + return; /* Pended, nothing to do. */
>> + if (ret != -EPERM) {
>> + req->out.h.error = ret;
>> + goto error;
>> + }
>> break;
>> }
>> case FUSE_FSYNC:
>> @@ -976,12 +1003,15 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster *pfc, struct fuse_req *req,
>> goto error;
>> }
>>
>> - ret = pcs_fuse_prep_rw(r);
>> - if (!ret)
>> + ret = pcs_fuse_prep_rw(r, ff);
>> + if (likely(!ret))
>> goto submit;
>> if (ret > 0)
>> - /* Pended, nothing to do. */
>> - return;
>> + return; /* Pended, nothing to do. */
>> + if (ret != -EPERM) {
>> + req->out.h.error = ret;
>> + goto error;
>
> Hm, strange thing in generic fuse code we fail immediate requests with EIO
> (see fuse_request_queue_background()), but there we finish them as successful.
No, here we are fail immediate requests with EIO too, if ret != -EPERM
that means ret == -EIO or another error.
> Also, I'm not sure, that EPERM is good value to use. IMO, it confuses a lot.
> Can't all pcs_fuse_prep_rw() fail with EIO?
EPERM is used only for those situations when the request is just nop for
vstorage, but we should completed it successfully. Previously,
pcs_fuse_prep_rw() used to return -1, I just replaced it to -EPERM.
But conceptually nothing has changed, pcs_fuse_prep_rw() will return -EIO in
case when ff_state became FUSE_S_FAIL_IMMEDIATELY, and request will be
immediately filed like in fuse_request_queue_background().
>> + }
>> break;
>> }
>> r->req.out.h.error = 0;
>> @@ -996,14 +1026,10 @@ error:
>> return;
>>
>> submit:
>> - spin_lock(&di->kq_lock);
>> - if (ff && test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state)) {
>> - spin_unlock(&di->kq_lock);
>> + if (!kqueue_insert(di, ff, req)) {
>> req->out.h.error = -EIO;
>> goto error;
>> }
>> - list_add_tail(&req->list, &di->kq);
>> - spin_unlock(&di->kq_lock);
>>
>> if (async)
>> pcs_cc_submit(ireq->cc, ireq);
>> @@ -1071,7 +1097,7 @@ static void _pcs_shrink_end(struct fuse_conn *fc, struct fuse_req *req)
>>
>> TRACE("resubmit %p\n", &r->req);
>> list_del_init(&ireq->list);
>> - pcs_fuse_submit(pfc, &r->req, NULL, true, false);
>> + pcs_fuse_submit(pfc, &r->req, r->req.ff, true, false);
>> }
>> }
>>
>>
>
More information about the Devel
mailing list