lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 15 Jan 2019 17:36:18 +0100
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Kirill Tkhai <ktkhai@...tuozzo.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] fuse: Replace page without copying in fuse_writepage_in_flight()

On Tue, Jan 15, 2019 at 5:14 PM Kirill Tkhai <ktkhai@...tuozzo.com> wrote:
>
> On 15.01.2019 18:39, Miklos Szeredi wrote:
> > On Mon, Nov 26, 2018 at 10:46 AM Kirill Tkhai <ktkhai@...tuozzo.com> wrote:
> >>
> >> It looks like we can optimize old_req page replacement
> >> and avoid copying by simple updating the request's page.
> >>
> >> Signed-off-by: Kirill Tkhai <ktkhai@...tuozzo.com>
> >> ---
> >>  fs/fuse/file.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> >> index c6650c68b31a..83b54b082c86 100644
> >> --- a/fs/fuse/file.c
> >> +++ b/fs/fuse/file.c
> >> @@ -1778,7 +1778,7 @@ static bool fuse_writepage_in_flight(struct fuse_req *new_req,
> >>         if (old_req->num_pages == 1 && old_req != first_req) {
> >>                 struct backing_dev_info *bdi = inode_to_bdi(page->mapping->host);
> >>
> >> -               copy_highpage(old_req->pages[0], page);
> >> +               swap(old_req->pages[0], page);
> >
> > This would mess up refcounting for all pages involved.   need to swap
> > with the temp page in new_req.    Fixed version in #for-next.
>
> You are sure, page is just a simple pointer, not struct **page.
> Then we would have had to change fuse_writepage_in_flight() to use ** pointer.

Using a struct page** would still have been broken, not because of
refcounting, but because of putting the wrong page into the request
(we do the temporary copy to avoid some issues with adding the page
cache page directly into the request)

Thanks,
Miklos

Powered by blists - more mailing lists