| Message ID | 20220218195739.585044-2-shr@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support sync buffered writes for io-uring | expand |
On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: > This adds a flags parameter to the __begin_write_begin_int() function. > This allows to pass flags down the stack. Still no.
On 2/18/22 11:59 AM, Matthew Wilcox wrote: > On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: >> This adds a flags parameter to the __begin_write_begin_int() function. >> This allows to pass flags down the stack. > > Still no. Currently block_begin_write_cache is expecting an aop_flag. Are you asking to first have a patch that replaces the existing aop_flag parameter with the gfp_t? and then modify this patch to directly use gfp flags?
On Fri, Feb 18, 2022 at 12:08:27PM -0800, Stefan Roesch wrote: > > > On 2/18/22 11:59 AM, Matthew Wilcox wrote: > > On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: > >> This adds a flags parameter to the __begin_write_begin_int() function. > >> This allows to pass flags down the stack. > > > > Still no. > > Currently block_begin_write_cache is expecting an aop_flag. Are you asking to There is no function by that name in Linus' tree. > first have a patch that replaces the existing aop_flag parameter with the gfp_t? > and then modify this patch to directly use gfp flags?
On 2/18/22 12:13 PM, Matthew Wilcox wrote: > On Fri, Feb 18, 2022 at 12:08:27PM -0800, Stefan Roesch wrote: >> >> >> On 2/18/22 11:59 AM, Matthew Wilcox wrote: >>> On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: >>>> This adds a flags parameter to the __begin_write_begin_int() function. >>>> This allows to pass flags down the stack. >>> >>> Still no. >> >> Currently block_begin_write_cache is expecting an aop_flag. Are you asking to > > There is no function by that name in Linus' tree. > >> first have a patch that replaces the existing aop_flag parameter with the gfp_t? >> and then modify this patch to directly use gfp flags? s/block_begin_write_cache/block_write_begin/
On Fri, Feb 18, 2022 at 12:14:50PM -0800, Stefan Roesch wrote: > > > On 2/18/22 12:13 PM, Matthew Wilcox wrote: > > On Fri, Feb 18, 2022 at 12:08:27PM -0800, Stefan Roesch wrote: > >> > >> > >> On 2/18/22 11:59 AM, Matthew Wilcox wrote: > >>> On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: > >>>> This adds a flags parameter to the __begin_write_begin_int() function. > >>>> This allows to pass flags down the stack. > >>> > >>> Still no. > >> > >> Currently block_begin_write_cache is expecting an aop_flag. Are you asking to > > > > There is no function by that name in Linus' tree. > > > >> first have a patch that replaces the existing aop_flag parameter with the gfp_t? > >> and then modify this patch to directly use gfp flags? > > s/block_begin_write_cache/block_write_begin/ I don't think there's any need to change the arguments to block_write_begin(). That's widely used and I don't think changing all the users is worth it. You don't seem to call it anywhere in this patch set. But having block_write_begin() translate the aop flags into gfp and fgp flags, yes. It can call pagecache_get_page() instead of grab_cache_page_write_begin(). And then you don't need to change grab_cache_page_write_begin() at all.
On 2/18/22 12:22 PM, Matthew Wilcox wrote: > On Fri, Feb 18, 2022 at 12:14:50PM -0800, Stefan Roesch wrote: >> >> >> On 2/18/22 12:13 PM, Matthew Wilcox wrote: >>> On Fri, Feb 18, 2022 at 12:08:27PM -0800, Stefan Roesch wrote: >>>> >>>> >>>> On 2/18/22 11:59 AM, Matthew Wilcox wrote: >>>>> On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: >>>>>> This adds a flags parameter to the __begin_write_begin_int() function. >>>>>> This allows to pass flags down the stack. >>>>> >>>>> Still no. >>>> >>>> Currently block_begin_write_cache is expecting an aop_flag. Are you asking to >>> >>> There is no function by that name in Linus' tree. >>> >>>> first have a patch that replaces the existing aop_flag parameter with the gfp_t? >>>> and then modify this patch to directly use gfp flags? >> >> s/block_begin_write_cache/block_write_begin/ > > I don't think there's any need to change the arguments to > block_write_begin(). That's widely used and I don't think changing > all the users is worth it. You don't seem to call it anywhere in this > patch set. > > But having block_write_begin() translate the aop flags into gfp > and fgp flags, yes. It can call pagecache_get_page() instead of > grab_cache_page_write_begin(). And then you don't need to change > grab_cache_page_write_begin() at all. That would still require adding a new aop flag (AOP_FLAG_NOWAIT). You are ok with that?
On Fri, Feb 18, 2022 at 12:25:41PM -0800, Stefan Roesch wrote: > > > On 2/18/22 12:22 PM, Matthew Wilcox wrote: > > On Fri, Feb 18, 2022 at 12:14:50PM -0800, Stefan Roesch wrote: > >> > >> > >> On 2/18/22 12:13 PM, Matthew Wilcox wrote: > >>> On Fri, Feb 18, 2022 at 12:08:27PM -0800, Stefan Roesch wrote: > >>>> > >>>> > >>>> On 2/18/22 11:59 AM, Matthew Wilcox wrote: > >>>>> On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: > >>>>>> This adds a flags parameter to the __begin_write_begin_int() function. > >>>>>> This allows to pass flags down the stack. > >>>>> > >>>>> Still no. > >>>> > >>>> Currently block_begin_write_cache is expecting an aop_flag. Are you asking to > >>> > >>> There is no function by that name in Linus' tree. > >>> > >>>> first have a patch that replaces the existing aop_flag parameter with the gfp_t? > >>>> and then modify this patch to directly use gfp flags? > >> > >> s/block_begin_write_cache/block_write_begin/ > > > > I don't think there's any need to change the arguments to > > block_write_begin(). That's widely used and I don't think changing > > all the users is worth it. You don't seem to call it anywhere in this > > patch set. > > > > But having block_write_begin() translate the aop flags into gfp > > and fgp flags, yes. It can call pagecache_get_page() instead of > > grab_cache_page_write_begin(). And then you don't need to change > > grab_cache_page_write_begin() at all. > > That would still require adding a new aop flag (AOP_FLAG_NOWAIT). > You are ok with that? No new AOP_FLAG. block_write_begin() does not get called with AOP_FLAG_NOWAIT in this series. You'd want to pass gfp flags to __block_write_begin_int instead of aop flags.
On 2/18/22 12:35 PM, Matthew Wilcox wrote: > On Fri, Feb 18, 2022 at 12:25:41PM -0800, Stefan Roesch wrote: >> >> >> On 2/18/22 12:22 PM, Matthew Wilcox wrote: >>> On Fri, Feb 18, 2022 at 12:14:50PM -0800, Stefan Roesch wrote: >>>> >>>> >>>> On 2/18/22 12:13 PM, Matthew Wilcox wrote: >>>>> On Fri, Feb 18, 2022 at 12:08:27PM -0800, Stefan Roesch wrote: >>>>>> >>>>>> >>>>>> On 2/18/22 11:59 AM, Matthew Wilcox wrote: >>>>>>> On Fri, Feb 18, 2022 at 11:57:27AM -0800, Stefan Roesch wrote: >>>>>>>> This adds a flags parameter to the __begin_write_begin_int() function. >>>>>>>> This allows to pass flags down the stack. >>>>>>> >>>>>>> Still no. >>>>>> >>>>>> Currently block_begin_write_cache is expecting an aop_flag. Are you asking to >>>>> >>>>> There is no function by that name in Linus' tree. >>>>> >>>>>> first have a patch that replaces the existing aop_flag parameter with the gfp_t? >>>>>> and then modify this patch to directly use gfp flags? >>>> >>>> s/block_begin_write_cache/block_write_begin/ >>> >>> I don't think there's any need to change the arguments to >>> block_write_begin(). That's widely used and I don't think changing >>> all the users is worth it. You don't seem to call it anywhere in this >>> patch set. >>> >>> But having block_write_begin() translate the aop flags into gfp >>> and fgp flags, yes. It can call pagecache_get_page() instead of >>> grab_cache_page_write_begin(). And then you don't need to change >>> grab_cache_page_write_begin() at all. >> >> That would still require adding a new aop flag (AOP_FLAG_NOWAIT). >> You are ok with that? > > No new AOP_FLAG. block_write_begin() does not get called with > AOP_FLAG_NOWAIT in this series. You'd want to pass gfp flags to > __block_write_begin_int instead of aop flags. v2 of the patch series is using AOP_FLAG_NOWAIT in block_write_begin(). Without introducing a new aop flag, how would I know in block_write_begin() that the request is a nowait async buffered write?
diff --git a/fs/buffer.c b/fs/buffer.c index 8e112b6bd371..6e6a69a12eed 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1970,7 +1970,8 @@ iomap_to_bh(struct inode *inode, sector_t block, struct buffer_head *bh, } int __block_write_begin_int(struct folio *folio, loff_t pos, unsigned len, - get_block_t *get_block, const struct iomap *iomap) + get_block_t *get_block, const struct iomap *iomap, + unsigned int flags) { unsigned from = pos & (PAGE_SIZE - 1); unsigned to = from + len; @@ -2058,7 +2059,7 @@ int __block_write_begin(struct page *page, loff_t pos, unsigned len, get_block_t *get_block) { return __block_write_begin_int(page_folio(page), pos, len, get_block, - NULL); + NULL, 0); } EXPORT_SYMBOL(__block_write_begin); @@ -2118,7 +2119,7 @@ int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len, if (!page) return -ENOMEM; - status = __block_write_begin(page, pos, len, get_block); + status = __block_write_begin_int(page_folio(page), pos, len, get_block, NULL, flags); if (unlikely(status)) { unlock_page(page); put_page(page); diff --git a/fs/internal.h b/fs/internal.h index 8590c973c2f4..7432df23f3ce 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -38,7 +38,8 @@ static inline int emergency_thaw_bdev(struct super_block *sb) * buffer.c */ int __block_write_begin_int(struct folio *folio, loff_t pos, unsigned len, - get_block_t *get_block, const struct iomap *iomap); + get_block_t *get_block, const struct iomap *iomap, + unsigned int flags); /* * char_dev.c diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 6c51a75d0be6..47c519952725 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -646,7 +646,7 @@ static int iomap_write_begin(const struct iomap_iter *iter, loff_t pos, if (srcmap->type == IOMAP_INLINE) status = iomap_write_begin_inline(iter, folio); else if (srcmap->flags & IOMAP_F_BUFFER_HEAD) - status = __block_write_begin_int(folio, pos, len, NULL, srcmap); + status = __block_write_begin_int(folio, pos, len, NULL, srcmap, 0); else status = __iomap_write_begin(iter, pos, len, folio); @@ -979,7 +979,7 @@ static loff_t iomap_folio_mkwrite_iter(struct iomap_iter *iter, if (iter->iomap.flags & IOMAP_F_BUFFER_HEAD) { ret = __block_write_begin_int(folio, iter->pos, length, NULL, - &iter->iomap); + &iter->iomap, 0); if (ret) return ret; block_commit_write(&folio->page, 0, length);
This adds a flags parameter to the __begin_write_begin_int() function. This allows to pass flags down the stack. Signed-off-by: Stefan Roesch <shr@fb.com> --- fs/buffer.c | 7 ++++--- fs/internal.h | 3 ++- fs/iomap/buffered-io.c | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-)