linux-kernel - Re: [V9fs-developer] [PATCH 02/13] 9p: Tell the VFS that readpage was synchronous

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 linux-hardening linux-cve-announce 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:   Fri, 18 Sep 2020 07:59:19 +0200
From:   Dominique Martinet <asmadeus@...ewreck.org>
To:     "Matthew Wilcox (Oracle)" <willy@...radead.org>
Cc:     linux-fsdevel@...r.kernel.org, linux-cifs@...r.kernel.org,
        Richard Weinberger <richard@....at>, ecryptfs@...r.kernel.org,
        linux-um@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, linux-mtd@...ts.infradead.org,
        v9fs-developer@...ts.sourceforge.net, ceph-devel@...r.kernel.org,
        linux-afs@...ts.infradead.org
Subject: Re: [V9fs-developer] [PATCH 02/13] 9p: Tell the VFS that readpage
 was synchronous

Matthew Wilcox (Oracle) wrote on Thu, Sep 17, 2020:
> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
> index cce9ace651a2..506ca0ba2ec7 100644
> --- a/fs/9p/vfs_addr.c
> +++ b/fs/9p/vfs_addr.c
> @@ -280,6 +280,10 @@ static int v9fs_write_begin(struct file *filp, struct address_space *mapping,
>  		goto out;
>  
>  	retval = v9fs_fid_readpage(v9inode->writeback_fid, page);
> +	if (retval == AOP_UPDATED_PAGE) {
> +		retval = 0;
> +		goto out;
> +	}

FWIW this is a change of behaviour; for some reason the code used to
loop back to grab_cache_page_write_begin() and bail out on
PageUptodate() I suppose; some sort of race check?
The whole pattern is a bit weird to me and 9p has no guarantee on
concurrent writes to a file with cache enabled (except that it will
corrupt something), so this part is fine with me.

What I'm curious about is the page used to be both unlocked and put, but
now isn't either and the return value hasn't changed for the caller to
make a difference on write_begin / I don't see any code change in the
vfs  to handle that.
What did I miss?

(FWIW at least cifs in the series has the same pattern change; didn't
check all of them)

Thanks,
-- 
Dominique