All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Damien Le Moal <dlemoal@kernel.org>
Cc: linux-fsdevel@vger.kernel.org,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] zonefs: use iomap for synchronous direct writes
Date: Thu, 1 Jun 2023 11:27:19 +0200	[thread overview]
Message-ID: <20230601092719.GA5774@lst.de> (raw)
In-Reply-To: <20230601082652.181695-1-dlemoal@kernel.org>

> +struct zonefs_bio {
> +	/* The target inode of the BIO */
> +	struct inode *inode;
> +
> +	/* For sync writes, the target write offset */
> +	u64 woffset;

Maybe spell out write_offset?

> +
> +static void zonefs_file_sync_write_dio_bio_end_io(struct bio *bio)
> +{
> +	struct zonefs_bio *zbio;
> +	struct zonefs_zone *z;
> +	sector_t wsector;
> +
> +	if (bio->bi_status != BLK_STS_OK)
> +		goto bio_end;
> +
> +	/*
> +	 * If the file zone was written underneath the file system, the zone
> +	 * append operation can still succedd (if the zone is not full) but
> +	 * the write append location will not be where we expect it to be.
> +	 * Check that we wrote where we intended to, that is, at z->z_wpoffset.
> +	 */
> +	zbio = zonefs_bio(bio);
> +	z = zonefs_inode_zone(zbio->inode);

I'd move thses to the lines where the variables are declared.

> +	wsector = z->z_sector + (zbio->woffset >> SECTOR_SHIFT);
> +	if (bio->bi_iter.bi_sector != wsector) {
> +		zonefs_warn(zbio->inode->i_sb,
> +			    "Invalid write sector %llu for zone at %llu\n",
> +			    bio->bi_iter.bi_sector, z->z_sector);
> +		bio->bi_status = BLK_STS_IOERR;
> +	}

Seems like all this is actually just debug code and could be conditional
and you could just use the normal bio pool otherwise?

> +static struct bio_set zonefs_file_write_dio_bio_set;
>  

  reply	other threads:[~2023-06-01  9:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01  8:26 [PATCH] zonefs: use iomap for synchronous direct writes Damien Le Moal
2023-06-01  9:27 ` Christoph Hellwig [this message]
2023-06-01  9:33   ` Damien Le Moal

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=20230601092719.GA5774@lst.de \
    --to=hch@lst.de \
    --cc=dlemoal@kernel.org \
    --cc=johannes.thumshirn@wdc.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.