Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181007232736.3780-1-martin@omnibond.com>
In-Reply-To: <20181007232736.3780-1-martin@omnibond.com>
From:   Mike Marshall <hubcap@omnibond.com>
Date:   Mon, 8 Oct 2018 16:43:36 -0400
Message-ID: <CAOg9mSTBRni_NVtTr2-W8g7e3LromGrpSww3MOJRNSw=Y_-oaA@mail.gmail.com>
Subject: Re: [PATCH 00/19] [V2] orangefs: page cache
To:     Martin Brandenburg <martin@omnibond.com>
Cc:     devel@lists.orangefs.org,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

I still hit writepages_callback not private! on xfstests generic/127...

-Mike
On Sun, Oct 7, 2018 at 7:27 PM Martin Brandenburg <martin@omnibond.com> wrote:
>
> V2... see https://marc.info/?l=linux-fsdevel&m=153721507330730&w=2
>
> One important change is the following, without which an unaligned write
> may end up written to the beginning of its page.  Surprisingly xfstests
> did not catch this.  This was caught by an invalidate_inode_pages2 call
> in read_iter (not part of this patch series) which exposed the bug.
>
> diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
> index 34b98d2ed377..cd1263c45bb2 100644
> --- a/fs/orangefs/inode.c
> +++ b/fs/orangefs/inode.c
> @@ -56,7 +54,7 @@ static int orangefs_writepage_locked(struct page *page,
>
>         bv.bv_page = page;
>         bv.bv_len = wlen;
> -       bv.bv_offset = 0;
> +       bv.bv_offset = off % PAGE_SIZE;
>         iov_iter_bvec(&iter, ITER_BVEC | WRITE, &bv, 1, wlen);
>
>         ret = wait_for_direct_io(ORANGEFS_IO_WRITE, inode, &off, &iter, wlen,
>
> The other big thing is an improved releasepage implementation and a
> launder_page implementation.
>
> We also size the writepages buffer based on the client core buffer.
> However I'm not happy with the memory allocation in writepages.
>
> Mike, I know you've had some trouble with the original series.  I'd like
> to know if this fixes that.
>
> Martin Brandenburg (19):
>   orangefs: implement xattr cache
>   orangefs: do not invalidate attributes on inode create
>   orangefs: simplify orangefs_inode_getattr interface
>   orangefs: update attributes rather than relying on server
>   orangefs: hold i_lock during inode_getattr
>   orangefs: set up and use backing_dev_info
>   orangefs: let setattr write to cached inode
>   orangefs: reorganize setattr functions to track attribute changes
>   orangefs: remove orangefs_readpages
>   orangefs: service ops done for writeback are not killable
>   orangefs: migrate to generic_file_read_iter
>   orangefs: implement writepage
>   orangefs: skip inode writeout if nothing to write
>   orangefs: write range tracking
>   orangefs: avoid fsync service operation on flush
>   orangefs: use kmem_cache for orangefs_write_request
>   orangefs: implement writepages
>   orangefs: use client-core buffer size to determine writepages count
>   orangefs: do writepages_work if a single page must be written
>
>  fs/orangefs/acl.c             |   4 +-
>  fs/orangefs/file.c            | 194 +++--------
>  fs/orangefs/inode.c           | 628 ++++++++++++++++++++++++++++------
>  fs/orangefs/namei.c           |  41 +--
>  fs/orangefs/orangefs-cache.c  |  24 +-
>  fs/orangefs/orangefs-kernel.h |  56 ++-
>  fs/orangefs/orangefs-mod.c    |  10 +-
>  fs/orangefs/orangefs-utils.c  | 181 +++++-----
>  fs/orangefs/super.c           |  38 +-
>  fs/orangefs/waitqueue.c       |  18 +-
>  fs/orangefs/xattr.c           | 104 ++++++
>  11 files changed, 893 insertions(+), 405 deletions(-)
>
> --
> 2.19.0
>