linux-ext4 - Re: [PATCH v7 11/11] ext4: introduce direct I/O write using iomap infrastructure

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]

Message-ID: <20191105203158.GA1739@bobrowski>
Date:   Wed, 6 Nov 2019 07:32:00 +1100
From:   Matthew Bobrowski <mbobrowski@...browski.org>
To:     Jan Kara <jack@...e.cz>
Cc:     tytso@....edu, adilger.kernel@...ger.ca,
        linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        riteshh@...ux.ibm.com
Subject: Re: [PATCH v7 11/11] ext4: introduce direct I/O write using iomap
 infrastructure

On Tue, Nov 05, 2019 at 02:59:32PM +0100, Jan Kara wrote:
> On Tue 05-11-19 23:02:39, Matthew Bobrowski wrote:
> > +	if (ret >= 0 && iov_iter_count(from)) {
> > +		ssize_t err;
> > +		loff_t endbyte;
> > +
> > +		offset = iocb->ki_pos;
> > +		err = ext4_buffered_write_iter(iocb, from);
> > +		if (err < 0)
> > +			return err;
> > +
> > +		/*
> > +		 * We need to ensure that the pages within the page cache for
> > +		 * the range covered by this I/O are written to disk and
> > +		 * invalidated. This is in attempt to preserve the expected
> > +		 * direct I/O semantics in the case we fallback to buffered I/O
> > +		 * to complete off the I/O request.
> > +		 */
> > +		ret += err;
> > +		endbyte = offset + ret - 1;
> 				   ^^ err here?
> 
> Otherwise you would write out and invalidate too much AFAICT - the 'offset'
> is position just before we fall back to buffered IO. Otherwise this hunk
> looks good to me.

Er, yes. That's right, it should rather be 'err' instead or else we
would write/invalidate too much. I actually had this originally, but I
must've muddled it up while rewriting this patch on my other computer.

Thanks for picking that up!

/M