All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-f2fs-devel@lists.sourceforge.net,
	LKML <linux-kernel@vger.kernel.org>,
	Lu Hongfei <luhongfei@vivo.com>, Yangtao Li <frank.li@vivo.com>
Subject: Re: [f2fs-dev] f2fs async buffered write patch
Date: Mon, 19 Jun 2023 23:16:20 -0700	[thread overview]
Message-ID: <ZJFENFDFVx++RmhA@google.com> (raw)
In-Reply-To: <1dc1a0f2-9be4-8ae0-da26-3c00c8a71b41@kernel.dk>

On 06/19, Jens Axboe wrote:
> Hi,
> 
> I came across this patch in a news posting:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=d618126911829523e35a61f4a5a4ad159b1b2c8d
> 
> which has me a bit worried. As far as I can tell, all that patch does is
> set FMODE_BUF_WASYNC, and then just hope that the lower layers handle
> the rest?
> 
> What happens if iocb->ki_flags & IOCB_NOWAIT is true, and now we do:
> 
> generic_perform_write(iocb, from)
> 	...
> 	->write_begin() <- does this block?
> 	...
> 	->write_end() <- or this one?
> 	...
> 	balance_dirty_pages_ratelimited() <- this one surely does...
> 
> If you look just one level down the latter to
> balance_dirty_pages_ratelimited_flags(), you'll even see the 'flags'
> argument documented there.
> 
> This looks pretty haphazard and cannot possibly work as-is, so please
> get this reverted until f2fs is converted to iomap, or IOCB_NOWAIT is
> handled by generic_perform_write() and below.

Thank you for pointing that out. It seems I haven't reviewed it carefully.
Hence I removed it from -next, and hope to have some time to convert iomap
soon.

Thanks,

> 
> -- 
> Jens Axboe


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: Yangtao Li <frank.li@vivo.com>, Lu Hongfei <luhongfei@vivo.com>,
	linux-f2fs-devel@lists.sourceforge.net,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: f2fs async buffered write patch
Date: Mon, 19 Jun 2023 23:16:20 -0700	[thread overview]
Message-ID: <ZJFENFDFVx++RmhA@google.com> (raw)
In-Reply-To: <1dc1a0f2-9be4-8ae0-da26-3c00c8a71b41@kernel.dk>

On 06/19, Jens Axboe wrote:
> Hi,
> 
> I came across this patch in a news posting:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=d618126911829523e35a61f4a5a4ad159b1b2c8d
> 
> which has me a bit worried. As far as I can tell, all that patch does is
> set FMODE_BUF_WASYNC, and then just hope that the lower layers handle
> the rest?
> 
> What happens if iocb->ki_flags & IOCB_NOWAIT is true, and now we do:
> 
> generic_perform_write(iocb, from)
> 	...
> 	->write_begin() <- does this block?
> 	...
> 	->write_end() <- or this one?
> 	...
> 	balance_dirty_pages_ratelimited() <- this one surely does...
> 
> If you look just one level down the latter to
> balance_dirty_pages_ratelimited_flags(), you'll even see the 'flags'
> argument documented there.
> 
> This looks pretty haphazard and cannot possibly work as-is, so please
> get this reverted until f2fs is converted to iomap, or IOCB_NOWAIT is
> handled by generic_perform_write() and below.

Thank you for pointing that out. It seems I haven't reviewed it carefully.
Hence I removed it from -next, and hope to have some time to convert iomap
soon.

Thanks,

> 
> -- 
> Jens Axboe

  reply	other threads:[~2023-06-20  6:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-19 20:43 [f2fs-dev] f2fs async buffered write patch Jens Axboe
2023-06-19 20:43 ` Jens Axboe
2023-06-20  6:16 ` Jaegeuk Kim [this message]
2023-06-20  6:16   ` Jaegeuk Kim
2023-06-20 13:09   ` [f2fs-dev] " Jens Axboe
2023-06-20 13:09     ` Jens Axboe
2023-06-26  7:31 ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
2023-06-26  7:31   ` Yangtao Li
2023-06-26 12:16   ` [f2fs-dev] " Matthew Wilcox
2023-06-26 12:16     ` Matthew Wilcox

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=ZJFENFDFVx++RmhA@google.com \
    --to=jaegeuk@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=frank.li@vivo.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luhongfei@vivo.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.