diff mbox series

[v2] FAT: use schedule_timeout_uninterruptible() instead of congestion_wait()

Message ID 163911016975.9928.6568675782275129@noble.neil.brown.name (mailing list archive)
State New, archived
Headers show
Series [v2] FAT: use schedule_timeout_uninterruptible() instead of congestion_wait() | expand

Commit Message

NeilBrown Dec. 10, 2021, 4:22 a.m. UTC
congestion_wait() in this context is just a sleep - block devices do not
in general support congestion signalling any more.

The goal here is to wait for any recently written data to get to
storage.  blkdev_issue_flush() is thought to be too expensive, so
replace congestion_wait() with an explicit timeout.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/fat/file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

OGAWA Hirofumi Dec. 11, 2021, 8:27 a.m. UTC | #1
"NeilBrown" <neilb@suse.de> writes:

> congestion_wait() in this context is just a sleep - block devices do not
> in general support congestion signalling any more.
>
> The goal here is to wait for any recently written data to get to
> storage.  blkdev_issue_flush() is thought to be too expensive, so
> replace congestion_wait() with an explicit timeout.

If just replace, the following looks better

	set_current_state(TASK_UNINTERRUPTIBLE);
	io_schedule_timeout(HZ/10);

Otherwise,

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/fat/file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fat/file.c b/fs/fat/file.c
> index 13855ba49cd9..2321fb3eded5 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c
> @@ -175,9 +175,9 @@ long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  static int fat_file_release(struct inode *inode, struct file *filp)
>  {
>  	if ((filp->f_mode & FMODE_WRITE) &&
> -	     MSDOS_SB(inode->i_sb)->options.flush) {
> +	    MSDOS_SB(inode->i_sb)->options.flush) {
>  		fat_flush_inodes(inode->i_sb, inode, NULL);
> -		congestion_wait(BLK_RW_ASYNC, HZ/10);
> +		schedule_timeout_uninterruptible(HZ/10);
>  	}
>  	return 0;
>  }
NeilBrown Dec. 13, 2021, 2:28 a.m. UTC | #2
On Sat, 11 Dec 2021, OGAWA Hirofumi wrote:
> "NeilBrown" <neilb@suse.de> writes:
> 
> > congestion_wait() in this context is just a sleep - block devices do not
> > in general support congestion signalling any more.
> >
> > The goal here is to wait for any recently written data to get to
> > storage.  blkdev_issue_flush() is thought to be too expensive, so
> > replace congestion_wait() with an explicit timeout.
> 
> If just replace, the following looks better
> 
> 	set_current_state(TASK_UNINTERRUPTIBLE);
> 	io_schedule_timeout(HZ/10);
> 
> Otherwise,
> 
> Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Thanks.
According to MAINTAINERS, I should send patches for this code to you,
with the implication (I assumed) that you would forwarded them upstream
if acceptable.
But the fact that you have send mt an Acked-By seems to suggest that you
won't be doing that.
To whom should I send this patch with your acked-by?

Thanks,
NeilBrown
OGAWA Hirofumi Dec. 13, 2021, 2:45 a.m. UTC | #3
"NeilBrown" <neilb@suse.de> writes:

> On Sat, 11 Dec 2021, OGAWA Hirofumi wrote:
>> "NeilBrown" <neilb@suse.de> writes:
>> 
>> > congestion_wait() in this context is just a sleep - block devices do not
>> > in general support congestion signalling any more.
>> >
>> > The goal here is to wait for any recently written data to get to
>> > storage.  blkdev_issue_flush() is thought to be too expensive, so
>> > replace congestion_wait() with an explicit timeout.
>> 
>> If just replace, the following looks better
>> 
>> 	set_current_state(TASK_UNINTERRUPTIBLE);
>> 	io_schedule_timeout(HZ/10);
>> 
>> Otherwise,
>> 
>> Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
>
> Thanks.
> According to MAINTAINERS, I should send patches for this code to you,
> with the implication (I assumed) that you would forwarded them upstream
> if acceptable.
> But the fact that you have send mt an Acked-By seems to suggest that you
> won't be doing that.
> To whom should I send this patch with your acked-by?

Ah, sorry. I have no repository. So FAT patches goes to linus tree via
akpm's help.

So "Cc: Andrew Morton <akpm@linux-foundation.org>" and my Acked-by
should work (or I will Cc as reply if need).

Thanks.
NeilBrown Dec. 13, 2021, 2:49 a.m. UTC | #4
On Mon, 13 Dec 2021, OGAWA Hirofumi wrote:
> "NeilBrown" <neilb@suse.de> writes:
> 
> > On Sat, 11 Dec 2021, OGAWA Hirofumi wrote:
> >> "NeilBrown" <neilb@suse.de> writes:
> >> 
> >> > congestion_wait() in this context is just a sleep - block devices do not
> >> > in general support congestion signalling any more.
> >> >
> >> > The goal here is to wait for any recently written data to get to
> >> > storage.  blkdev_issue_flush() is thought to be too expensive, so
> >> > replace congestion_wait() with an explicit timeout.
> >> 
> >> If just replace, the following looks better
> >> 
> >> 	set_current_state(TASK_UNINTERRUPTIBLE);
> >> 	io_schedule_timeout(HZ/10);
> >> 
> >> Otherwise,
> >> 
> >> Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> >
> > Thanks.
> > According to MAINTAINERS, I should send patches for this code to you,
> > with the implication (I assumed) that you would forwarded them upstream
> > if acceptable.
> > But the fact that you have send mt an Acked-By seems to suggest that you
> > won't be doing that.
> > To whom should I send this patch with your acked-by?
> 
> Ah, sorry. I have no repository. So FAT patches goes to linus tree via
> akpm's help.
> 
> So "Cc: Andrew Morton <akpm@linux-foundation.org>" and my Acked-by
> should work (or I will Cc as reply if need).

Will do, thanks.

NeilBrown
diff mbox series

Patch

diff --git a/fs/fat/file.c b/fs/fat/file.c
index 13855ba49cd9..2321fb3eded5 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -175,9 +175,9 @@  long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 static int fat_file_release(struct inode *inode, struct file *filp)
 {
 	if ((filp->f_mode & FMODE_WRITE) &&
-	     MSDOS_SB(inode->i_sb)->options.flush) {
+	    MSDOS_SB(inode->i_sb)->options.flush) {
 		fat_flush_inodes(inode->i_sb, inode, NULL);
-		congestion_wait(BLK_RW_ASYNC, HZ/10);
+		schedule_timeout_uninterruptible(HZ/10);
 	}
 	return 0;
 }