linux-kernel - Re: [PATCH v2 2/4] fd/direct-io: introduce should

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:	Wed, 28 Jan 2015 00:05:53 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	linux-kernel@...r.kernel.org,
	Dave Kleikamp <dave.kleikamp@...cle.com>,
	Jens Axboe <axboe@...nel.dk>, Zach Brown <zab@...bo.net>,
	Maxim Patlasov <mpatlasov@...allels.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Benjamin LaHaise <bcrl@...ck.org>,
	Omar Sandoval <osandov@...ndov.com>,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] fd/direct-io: introduce should_dirty for kernel aio

On 1/25/15, Christoph Hellwig <hch@...radead.org> wrote:
> On Tue, Jan 13, 2015 at 11:44:46PM +0800, Ming Lei wrote:
>>
>> -	if (dio->is_async && dio->rw == READ)
>> +	if (dio->is_async && dio->rw == READ && dio->should_dirty)
>>  		bio_set_pages_dirty(bio);
>>
>>  	if (sdio->submit_io)
>> @@ -463,13 +464,14 @@ static int dio_bio_complete(struct dio *dio, struct
>> bio *bio)
>>  	if (!uptodate)
>>  		dio->io_error = -EIO;
>>
>> -	if (dio->is_async && dio->rw == READ) {
>> +	if (dio->is_async && dio->rw == READ && dio->should_dirty) {
>
> I'd rather have a ->should_dirrty flag that means we need to call
> bio_check_pages_dirty, And set that either if we have a kernel
> iovec/bvec or dio->is_async && dio->rw == READ.

It may not work for loop because the page may not become dirty
until the read dio is completed, so it just means the caller totally
handles the page dirty if I/O is from kernel.

>
> But why would we even set this if writing from an iovec?

Or just rename it as ->from_user?

>>  		bio_check_pages_dirty(bio);	/* transfers ownership */
>>  	} else {
>>  		bio_for_each_segment_all(bvec, bio, i) {
>>  			struct page *page = bvec->bv_page;
>>
>> -			if (dio->rw == READ && !PageCompound(page))
>> +			if (dio->rw == READ && !PageCompound(page) &&
>> +					dio->should_dirty)
>>  				set_page_dirty_lock(page);
>
> And this unk could also use some explanation.

This is same, just don't handle page dirty for kernel dio pages.

Thanks,
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/