Re: [PATCH 5/8] nowait aio: return on congested block device

From: Jan Kara <jack@suse.cz>
To: Goldwyn Rodrigues <rgoldwyn@suse.de>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel@vger.kernel.org, jack@suse.com,
	linux-block@vger.kernel.org, linux-btrfs@vger.kernel.org,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
	sagi@grimberg.me, avi@scylladb.com, axboe@kernel.dk,
	linux-api@vger.kernel.org, willy@infradead.org,
	tom.leiming@gmail.com, Goldwyn Rodrigues <rgoldwyn@suse.com>
Subject: Re: [PATCH 5/8] nowait aio: return on congested block device
Date: Thu, 20 Apr 2017 15:43:56 +0200	[thread overview]
Message-ID: <20170420134356.GD22135@quack2.suse.cz> (raw)
In-Reply-To: <d3467259-675a-2723-7cf9-d4f2f067d634@suse.de>

On Wed 19-04-17 10:21:39, Goldwyn Rodrigues wrote:
> 
> 
> On 04/19/2017 01:45 AM, Christoph Hellwig wrote:
> > 
> >> +	if (bio->bi_opf & REQ_NOWAIT) {
> >> +		if (!blk_queue_nowait(q)) {
> >> +			err = -EOPNOTSUPP;
> >> +			goto end_io;
> >> +		}
> >> +		if (!(bio->bi_opf & REQ_SYNC)) {
> > 
> > I don't understand this check at all..
> 
> It is to ensure at the block layer that NOWAIT comes only for DIRECT
> calls only. I should probably change it to REQ_SYNC | REQ_IDLE.

Ouch. Checking 'REQ_SYNC' for this is

a) unreliable hack
b) layering violation

You just don't care why someone marked bio with REQ_NOWAIT at this place.
Just obey the request if you can, return error if you cannot, but advisory
REQ_SYNC or REQ_IDLE flags have nothing to do with the ability of the block
layer to submit the bio without blocking...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR