Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Kirill Tkhai <ktkhai@virtuozzo.com>,
	axboe@kernel.dk, bob.liu@oracle.com, agk@redhat.com,
	snitzer@redhat.com, dm-devel@redhat.com, song@kernel.org,
	tytso@mit.edu, adilger.kernel@dilger.ca,
	Chaitanya.Kulkarni@wdc.com, ming.lei@redhat.com, osandov@fb.com,
	jthumshirn@suse.de, minwoo.im.dev@gmail.com,
	damien.lemoal@wdc.com, andrea.parri@amarulasolutions.com,
	hare@suse.com, tj@kernel.org, ajay.joshi@wdc.com,
	sagi@grimberg.me, dsterba@suse.com, bvanassche@acm.org,
	dhowells@redhat.com, asml.silence@gmail.com,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 0/6] block: Introduce REQ_ALLOCATE flag for REQ_OP_WRITE_ZEROES
Date: Wed, 25 Mar 2020 09:26:56 -0700
Message-ID: <20200325162656.GJ29351@magnolia> (raw)
In-Reply-To: <yq1tv2k8pjn.fsf@oracle.com>

On Thu, Mar 19, 2020 at 09:03:40AM -0400, Martin K. Petersen wrote:
> 
> Christoph,
> 
> >> Some comments? Some requests for reworking? Some personal reasons to
> >> ignore my patches?
> >
> > I'm still completely opposed to the magic overloading using a flag.
> > That is just a bad design waiting for trouble to happen.
> 
> The observation was that Kirill's original patch set was a line-for-line
> carbon copy of the write zeroes handling throughout the stack. The only
> difference between the two was at the bottom. Instead of duplicating all
> that code it seemed cleaner to use shared plumbing since these
> operations need to be split and merged exactly the same way in the block
> layer.
> 
> Also, we already have REQ_NOUNMAP, not sure why an additional handling
> flag would lead to trouble? Note that I suggested renaming
> REQ_OP_WRITE_ZEROES to something else to separate the semantics from the
> plumbing.
> 
> We need to be able to express:
> 
>  - zero & allocate block range (REQ_OP_WRITE_ZEROES, REQ_NOUNMAP)
>  - zero & deallocate block range (REQ_OP_WRITE_ZEROES, !REQ_NOUNMAP)
>  - allocate block range (?, don't care about zeroing)
>  - deallocate block range (REQ_OP_DISCARD, don't care about zeroing)
> 
> It just seems like a full-fledged REQ_OP_ALLOCATE is an overkill to fill
> that gap.
> 
> That said, I do think that we have traditionally put emphasis on the
> wrong part of these operations. All we ever talk about wrt. discard and
> friends is the zeroing aspect. But I actually think that, semantically,
> the act of allocating and deallocating blocks is more important. And
> that zeroing is an optional second order effect of those operations. So
> if we could go back in time and nuke multi-range DSM TRIM/UNMAP, I would
> like to have REQ_OP_ALLOCATE/REQ_OP_DEALLOCATE with an optional REQ_ZERO
> flag. I think that would be cleaner. I have a much easier time wrapping
> my head around "allocate this block and zero it if you can" than "zero
> this block and do not deallocate it". But maybe that's just me.

I'd love to transition to that.  My brain is not good at following all
the inverse logic that NOUNMAP spread everywhere.  I have a difficult
time following what the blockdev fallocate code does, which is sad since
hch and I are the primary stuckees^Wmeddlers^Wauthors of that function. :/

--D

> -- 
> Martin K. Petersen	Oracle Linux Engineering

  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-13  7:39 Kirill Tkhai
2020-02-13  7:39 ` [PATCH v7 1/6] block: Add @flags argument to bdev_write_zeroes_sectors() Kirill Tkhai
2020-02-13  7:39 ` [PATCH v7 2/6] block: Pass op_flags into blk_queue_get_max_sectors() Kirill Tkhai
2020-02-13  7:39 ` [PATCH v7 3/6] block: Introduce blk_queue_get_max_write_zeroes_sectors() Kirill Tkhai
2020-02-13  7:39 ` [PATCH v7 4/6] block: Add support for REQ_ALLOCATE flag Kirill Tkhai
2020-02-13  7:39 ` [PATCH v7 5/6] block: Add blk_queue_max_allocate_sectors() Kirill Tkhai
2020-02-13  7:39 ` [PATCH v7 6/6] loop: Add support for REQ_ALLOCATE Kirill Tkhai
2020-02-13 18:11   ` Darrick J. Wong
2020-02-13 20:07     ` Kirill Tkhai
2020-02-13  7:55 ` [PATCH v7 0/6] block: Introduce REQ_ALLOCATE flag for REQ_OP_WRITE_ZEROES Kirill Tkhai
2020-03-06  9:11   ` Kirill Tkhai
2020-03-13 13:08     ` Kirill Tkhai
2020-03-19 10:28       ` Christoph Hellwig
2020-03-19 10:42         ` Kirill Tkhai
2020-03-19 13:03         ` Martin K. Petersen
2020-03-25 16:26           ` Darrick J. Wong [this message]
2020-03-25 16:32             ` Christoph Hellwig
2020-03-25 17:23               ` Martin K. Petersen
2020-03-26  9:29                 ` Christoph Hellwig
2020-03-26 14:34                   ` Martin K. Petersen
2020-03-26 14:45                     ` Christoph Hellwig
2020-03-26 16:48                       ` Chaitanya Kulkarni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200325162656.GJ29351@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=Chaitanya.Kulkarni@wdc.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=agk@redhat.com \
    --cc=ajay.joshi@wdc.com \
    --cc=andrea.parri@amarulasolutions.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=bob.liu@oracle.com \
    --cc=bvanassche@acm.org \
    --cc=damien.lemoal@wdc.com \
    --cc=dhowells@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=dsterba@suse.com \
    --cc=hare@suse.com \
    --cc=hch@infradead.org \
    --cc=jthumshirn@suse.de \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=minwoo.im.dev@gmail.com \
    --cc=osandov@fb.com \
    --cc=sagi@grimberg.me \
    --cc=snitzer@redhat.com \
    --cc=song@kernel.org \
    --cc=tj@kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org
	public-inbox-index linux-block

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git