linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Hillf Danton <hdanton@sina.com>
Cc: syzbot <syzbot+538d1957ce178382a394@syzkaller.appspotmail.com>,
	io-uring@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
	viro@zeniv.linux.org.uk
Subject: Re: INFO: task hung in io_queue_file_removal
Date: Sun, 22 Mar 2020 14:17:36 -0600	[thread overview]
Message-ID: <5f48249b-19f9-6c18-b849-d77db0b2f247@kernel.dk> (raw)
In-Reply-To: <20200322020601.5136-1-hdanton@sina.com>

On 3/21/20 8:06 PM, Hillf Danton wrote:
> 
> On Sat, 21 Mar 2020 14:03:24 -0600 Jens Axboe wrote:
>>
>> On 3/21/20 6:38 AM, Hillf Danton wrote:
>>>
>>> Flush work before waiting for completion.
>>>
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -5823,8 +5823,8 @@ static bool io_queue_file_removal(struct
>>>  
>>>  	if (pfile == &pfile_stack) {
>>>  		percpu_ref_switch_to_atomic(&data->refs, io_atomic_switch);
>>> -		wait_for_completion(&done);
>>>  		flush_work(&data->ref_work);
>>> +		wait_for_completion(&done);
>>>  		return false;
>>>  	}
>>>  
>>> --
>>>
>>> And perhaps a tiny cleanup: no deed to wait for completion as
>>> flushing work itself will wait until the work is done.
>>
>> Care to send this version as a real patch? Seems kind of pointless to
>> just do the above change with that in mind. And then at the same time
>> turn ->done into ->do_file_put or something, and make it a bool.
> 
> Have trouble making a patch with the ideas in your mind all folded in so
> it may be better that you do it this time leaving me a chance to learn
> a lesson.

Maybe my explanation wasn't quite clear! What I meant was that since
we're no longer using pfile->done, turn that ->done into a ->needs_kfree
or something, and make that a bool. So basically the same patch as the
one you posted, just making that naming (and type) change as well.

Does that help? Would prefer if you sent a patch, you already did
99% of the hard work.

-- 
Jens Axboe


  parent reply	other threads:[~2020-03-22 20:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200321123827.15256-1-hdanton@sina.com>
2020-03-21 20:03 ` INFO: task hung in io_queue_file_removal Jens Axboe
     [not found] ` <20200322020601.5136-1-hdanton@sina.com>
2020-03-22 20:17   ` Jens Axboe [this message]
2020-03-21  5:50 syzbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5f48249b-19f9-6c18-b849-d77db0b2f247@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=hdanton@sina.com \
    --cc=io-uring@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+538d1957ce178382a394@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).