Re: [Cluster-devel] [PATCH 1/3] fs/buffer.c: add new api to allow eof writeback

[Junxiao Bi <junxiao.bi@xxxxxxxxxx>]

On 4/29/21 10:14 AM, Andreas Gruenbacher wrote:

> Junxiao,
>
> On Tue, Apr 27, 2021 at 4:44 AM Junxiao Bi <junxiao.bi@xxxxxxxxxx> wrote:
> > When doing truncate/fallocate for some filesytem like ocfs2, it
> > will zero some pages that are out of inode size and then later
> > update the inode size, so it needs this api to writeback eof
> > pages.
> is this in reaction to Jan's "[PATCH 0/12 v4] fs: Hole punch vs page
> cache filling races" patch set [*]? It doesn't look like the kind of
> patch Christoph would be happy with.

Thank you for pointing the patch set. I think that is fixing a different 
issue.

The issue here is when extending file size with fallocate/truncate, if 
the original inode size

is in the middle of the last cluster block(1M), eof part will be zeroed 
with buffer write first,

and then new inode size is updated, so there is a window that dirty 
pages is out of inode size,

if writeback is kicked in, block_write_full_page will drop all those eof 
pages.

I guess gfs2 has the similar issue?

I think it would be good to provide an api that allowed eof write back. 
If this is not good,

do you have any advise how to improve/fix it?

Thanks,

Junxiao.


> Thanks,
> Andreas
>
> [*] https://lore.kernel.org/linux-fsdevel/20210423171010.12-1-jack@xxxxxxx/
>
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Junxiao Bi <junxiao.bi@xxxxxxxxxx>
> > ---
> >   fs/buffer.c                 | 14 +++++++++++---
> >   include/linux/buffer_head.h |  3 +++
> >   2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/buffer.c b/fs/buffer.c
> > index 0cb7ffd4977c..802f0bacdbde 100644
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -1709,9 +1709,9 @@ static struct buffer_head *create_page_buffers(struct page *page, struct inode *
> >    * WB_SYNC_ALL, the writes are posted using REQ_SYNC; this
> >    * causes the writes to be flagged as synchronous writes.
> >    */
> > -int __block_write_full_page(struct inode *inode, struct page *page,
> > +int __block_write_full_page_eof(struct inode *inode, struct page *page,
> >                          get_block_t *get_block, struct writeback_control *wbc,
> > -                       bh_end_io_t *handler)
> > +                       bh_end_io_t *handler, bool eof_write)
> >   {
> >          int err;
> >          sector_t block;
> > @@ -1746,7 +1746,7 @@ int __block_write_full_page(struct inode *inode, struct page *page,
> >           * handle any aliases from the underlying blockdev's mapping.
> >           */
> >          do {
> > -               if (block > last_block) {
> > +               if (block > last_block && !eof_write) {
> >                          /*
> >                           * mapped buffers outside i_size will occur, because
> >                           * this page can be outside i_size when there is a
> > @@ -1871,6 +1871,14 @@ int __block_write_full_page(struct inode *inode, struct page *page,
> >          unlock_page(page);
> >          goto done;
> >   }
> > +EXPORT_SYMBOL(__block_write_full_page_eof);
> > +
> > +int __block_write_full_page(struct inode *inode, struct page *page,
> > +                       get_block_t *get_block, struct writeback_control *wbc,
> > +                       bh_end_io_t *handler)
> > +{
> > +       return __block_write_full_page_eof(inode, page, get_block, wbc, handler, false);
> > +}
> >   EXPORT_SYMBOL(__block_write_full_page);
> >
> >   /*
> > diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> > index 6b47f94378c5..5da15a1ba15c 100644
> > --- a/include/linux/buffer_head.h
> > +++ b/include/linux/buffer_head.h
> > @@ -221,6 +221,9 @@ int block_write_full_page(struct page *page, get_block_t *get_block,
> >   int __block_write_full_page(struct inode *inode, struct page *page,
> >                          get_block_t *get_block, struct writeback_control *wbc,
> >                          bh_end_io_t *handler);
> > +int __block_write_full_page_eof(struct inode *inode, struct page *page,
> > +                       get_block_t *get_block, struct writeback_control *wbc,
> > +                       bh_end_io_t *handler, bool eof_write);
> >   int block_read_full_page(struct page*, get_block_t*);
> >   int block_is_partially_uptodate(struct page *page, unsigned long from,
> >                                  unsigned long count);
> > --
> > 2.24.3 (Apple Git-128)