diff mbox series

[17/17] iomap: remove IOMAP_F_ZONE_APPEND

Message ID 20220901074216.1849941-18-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [01/17] block: export bio_split_rw | expand

Commit Message

Christoph Hellwig Sept. 1, 2022, 7:42 a.m. UTC
No users left now that btrfs takes REQ_OP_WRITE bios from iomap and
splits and converts them to REQ_OP_ZONE_APPEND internally.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/direct-io.c  | 10 ++--------
 include/linux/iomap.h |  1 -
 2 files changed, 2 insertions(+), 9 deletions(-)

Comments

Johannes Thumshirn Sept. 1, 2022, 10:46 a.m. UTC | #1
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Damien Le Moal Sept. 2, 2022, 1:38 a.m. UTC | #2
On 9/1/22 16:42, Christoph Hellwig wrote:
> No users left now that btrfs takes REQ_OP_WRITE bios from iomap and
> splits and converts them to REQ_OP_ZONE_APPEND internally.

Hu... I wanted to use that for zonefs for doing ZONE APPEND with AIOs...
Need to revisit that code anyway, so fine for now.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/iomap/direct-io.c  | 10 ++--------
>  include/linux/iomap.h |  1 -
>  2 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 4eb559a16c9ed..9e883a9f80388 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -217,16 +217,10 @@ static inline blk_opf_t iomap_dio_bio_opflags(struct iomap_dio *dio,
>  {
>  	blk_opf_t opflags = REQ_SYNC | REQ_IDLE;
>  
> -	if (!(dio->flags & IOMAP_DIO_WRITE)) {
> -		WARN_ON_ONCE(iomap->flags & IOMAP_F_ZONE_APPEND);
> +	if (!(dio->flags & IOMAP_DIO_WRITE))
>  		return REQ_OP_READ;
> -	}
> -
> -	if (iomap->flags & IOMAP_F_ZONE_APPEND)
> -		opflags |= REQ_OP_ZONE_APPEND;
> -	else
> -		opflags |= REQ_OP_WRITE;
>  
> +	opflags |= REQ_OP_WRITE;
>  	if (use_fua)
>  		opflags |= REQ_FUA;
>  	else
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 238a03087e17e..ee6d511ef29dd 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -55,7 +55,6 @@ struct vm_fault;
>  #define IOMAP_F_SHARED		0x04
>  #define IOMAP_F_MERGED		0x08
>  #define IOMAP_F_BUFFER_HEAD	0x10
> -#define IOMAP_F_ZONE_APPEND	0x20
>  
>  /*
>   * Flags set by the core iomap code during operations:
Christoph Hellwig Sept. 5, 2022, 6:50 a.m. UTC | #3
On Fri, Sep 02, 2022 at 10:38:50AM +0900, Damien Le Moal wrote:
> On 9/1/22 16:42, Christoph Hellwig wrote:
> > No users left now that btrfs takes REQ_OP_WRITE bios from iomap and
> > splits and converts them to REQ_OP_ZONE_APPEND internally.
> 
> Hu... I wanted to use that for zonefs for doing ZONE APPEND with AIOs...
> Need to revisit that code anyway, so fine for now.

We could resurrect it.  But I suspect that you're better off doing
what btrfs does here - let iomap submit a write bio and then split
it in the submit_bio hook.
Damien Le Moal Sept. 5, 2022, 6:57 a.m. UTC | #4
On 9/5/22 15:50, Christoph Hellwig wrote:
> On Fri, Sep 02, 2022 at 10:38:50AM +0900, Damien Le Moal wrote:
>> On 9/1/22 16:42, Christoph Hellwig wrote:
>>> No users left now that btrfs takes REQ_OP_WRITE bios from iomap and
>>> splits and converts them to REQ_OP_ZONE_APPEND internally.
>>
>> Hu... I wanted to use that for zonefs for doing ZONE APPEND with AIOs...
>> Need to revisit that code anyway, so fine for now.
> 
> We could resurrect it.  But I suspect that you're better off doing
> what btrfs does here - let iomap submit a write bio and then split
> it in the submit_bio hook.

Nope, we cannot do that for zonefs as the data mapping is implied directly
from the written offset (no metadata) so we cannot split an async write
into multiple zone append BIOs, since the single aio write data may end up
all mingled due to possible reordering of the fragment BIOs.

But the need for a split can be checked before submission and an error
returned if the aio write is too large. So this method of using the
submit_bio hook still works and will simply turn a write into a zone
append if the file was open with O_APPEND.
Josef Bacik Sept. 7, 2022, 9:18 p.m. UTC | #5
On Thu, Sep 01, 2022 at 10:42:16AM +0300, Christoph Hellwig wrote:
> No users left now that btrfs takes REQ_OP_WRITE bios from iomap and
> splits and converts them to REQ_OP_ZONE_APPEND internally.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 4eb559a16c9ed..9e883a9f80388 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -217,16 +217,10 @@  static inline blk_opf_t iomap_dio_bio_opflags(struct iomap_dio *dio,
 {
 	blk_opf_t opflags = REQ_SYNC | REQ_IDLE;
 
-	if (!(dio->flags & IOMAP_DIO_WRITE)) {
-		WARN_ON_ONCE(iomap->flags & IOMAP_F_ZONE_APPEND);
+	if (!(dio->flags & IOMAP_DIO_WRITE))
 		return REQ_OP_READ;
-	}
-
-	if (iomap->flags & IOMAP_F_ZONE_APPEND)
-		opflags |= REQ_OP_ZONE_APPEND;
-	else
-		opflags |= REQ_OP_WRITE;
 
+	opflags |= REQ_OP_WRITE;
 	if (use_fua)
 		opflags |= REQ_FUA;
 	else
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 238a03087e17e..ee6d511ef29dd 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -55,7 +55,6 @@  struct vm_fault;
 #define IOMAP_F_SHARED		0x04
 #define IOMAP_F_MERGED		0x08
 #define IOMAP_F_BUFFER_HEAD	0x10
-#define IOMAP_F_ZONE_APPEND	0x20
 
 /*
  * Flags set by the core iomap code during operations: