linux-kernel - Re: [PATCH v3 4/4] block: loop: support DIO & AIO

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: <CACVXFVMN9EuPP+m0HWjatrXKufB4qokk7qRYQjhEz87ZjL53iQ@mail.gmail.com>
Date:	Thu, 7 May 2015 20:32:39 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Linux Kernel Mailing List <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>,
	Tejun Heo <tj@...nel.org>, util-linux@...r.kernel.org
Subject: Re: [PATCH v3 4/4] block: loop: support DIO & AIO

On Thu, May 7, 2015 at 3:24 PM, Christoph Hellwig <hch@...radead.org> wrote:
>> @@ -441,6 +500,12 @@ static void do_loop_switch(struct loop_device *lo, struct switch_request *p)
>>               mapping->host->i_bdev->bd_block_size : PAGE_SIZE;
>>       lo->old_gfp_mask = mapping_gfp_mask(mapping);
>>       mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
>> +
>> +     lo->support_dio = mapping->a_ops && mapping->a_ops->direct_IO;
>> +     if (lo->support_dio)
>> +             lo->use_aio = true;
>> +     else
>> +             lo->use_aio = false;
>
> We need an explicit userspace op-in for this.  For one direct I/O can't

Actually this patch is one simplified version, and my old version
has exported two sysfs files(use_aio, use_dio) which can control
if direct IO or AIO is used but only AIO is enabled if DIO is set. Finally
I think it isn't necessary because dio/aio works well from the tests,
and userspace shouldn't care if it is AIO or not if the performance
is good.

> handle sub-sector size access and people use the loop device as a
> workaround for that.

Yes, user can do that, could you explain a bit what the problem is?

> Second this doesn't give anyone seeing negative
> results from aio a way to disable it easily.

If there is the requirement, we can export the control interface via
sysfs files easily, but I suggest to not do that from the start for sake
of simplicity.

>
> It think the best way is to require a setup time flag, but enable it to
> on in losetup versions that know about it.

Could you explain a bit why we need losetup involved?

>
>> @@ -761,6 +826,13 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>>       if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
>>               blk_queue_flush(lo->lo_queue, REQ_FLUSH);
>>
>> +     /* use aio if it is possible */
>> +     lo->support_dio = mapping->a_ops && mapping->a_ops->direct_IO;
>> +     if (lo->support_dio)
>> +             lo->use_aio = true;
>> +     else
>> +             lo->use_aio = false;
>
> Please always factor out checks like this insted of duplicating them.

OK, will do that.


Thanks,
Ming
--
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/