2023-05-10 05:23:58

by Andreas Dilger

[permalink] [raw]
Subject: [PATCH] ext2fs: don't retry discard/zeroout repeatedly

Call safe_getenv(UNIX_IO_NOZEROOUT) once when the device is
opened and set CHANNEL_FLAG_NOZEROOUT if present instead of
getting uid/euid/getenv every time unix_zeroout() is called.

For unix_discard() and unix_zeroout() don't continue to call
them if the block device doesn't support these operations.

Signed-off-by: Andreas Dilger <[email protected]>
---
lib/ext2fs/ext2_io.h | 4 +++-
lib/ext2fs/unix_io.c | 43 ++++++++++++++++++++++++++-----------------
2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/lib/ext2fs/ext2_io.h b/lib/ext2fs/ext2_io.h
index 679184e393f1..27eaaf1be354 100644
--- a/lib/ext2fs/ext2_io.h
+++ b/lib/ext2fs/ext2_io.h
@@ -34,6 +34,8 @@ typedef struct struct_io_stats *io_stats;
#define CHANNEL_FLAGS_DISCARD_ZEROES 0x02
#define CHANNEL_FLAGS_BLOCK_DEVICE 0x04
#define CHANNEL_FLAGS_THREADS 0x08
+#define CHANNEL_FLAGS_NODISCARD 0x10
+#define CHANNEL_FLAGS_NOZEROOUT 0x20

#define io_channel_discard_zeroes_data(i) (i->flags & CHANNEL_FLAGS_DISCARD_ZEROES)

@@ -57,7 +59,7 @@ struct struct_io_channel {
int actual_bytes_written,
errcode_t error);
int refcount;
- int flags;
+ unsigned int flags;
long reserved[14];
void *private_data;
void *app_data;
diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 3171c7368feb..33c5d568656c 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -761,6 +761,9 @@ static errcode_t unix_open_channel(const char *name, int fd,
io->refcount = 1;
io->flags = 0;

+ if (safe_getenv("UNIX_IO_NOZEROOUT"))
+ io->flags |= CHANNEL_FLAGS_NOZEROOUT;
+
memset(data, 0, sizeof(struct unix_private_data));
data->magic = EXT2_ET_MAGIC_UNIX_IO_CHANNEL;
data->io_stats.num_fields = 2;
@@ -783,20 +786,19 @@ static errcode_t unix_open_channel(const char *name, int fd,
* zero.
*/
if (ext2fs_fstat(data->dev, &st) == 0) {
- if (ext2fsP_is_disk_device(st.st_mode))
- io->flags |= CHANNEL_FLAGS_BLOCK_DEVICE;
- else
- io->flags |= CHANNEL_FLAGS_DISCARD_ZEROES;
- }
-
+ if (ext2fsP_is_disk_device(st.st_mode)) {
#ifdef BLKDISCARDZEROES
- {
- int zeroes = 0;
- if (ioctl(data->dev, BLKDISCARDZEROES, &zeroes) == 0 &&
- zeroes)
+ int zeroes = 0;
+
+ if (ioctl(data->dev, BLKDISCARDZEROES, &zeroes) == 0 &&
+ zeroes)
+ io->flags |= CHANNEL_FLAGS_DISCARD_ZEROES;
+#endif
+ io->flags |= CHANNEL_FLAGS_BLOCK_DEVICE;
+ } else {
io->flags |= CHANNEL_FLAGS_DISCARD_ZEROES;
+ }
}
-#endif

#if defined(__CYGWIN__)
/*
@@ -1344,12 +1346,15 @@ static errcode_t unix_discard(io_channel channel, unsigned long long block,
unsigned long long count)
{
struct unix_private_data *data;
- int ret;
+ int ret = EOPNOTSUPP;

EXT2_CHECK_MAGIC(channel, EXT2_ET_MAGIC_IO_CHANNEL);
data = (struct unix_private_data *) channel->private_data;
EXT2_CHECK_MAGIC(data, EXT2_ET_MAGIC_UNIX_IO_CHANNEL);

+ if (channel->flags & CHANNEL_FLAGS_NODISCARD)
+ goto unimplemented;
+
if (channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE) {
#ifdef BLKDISCARD
__u64 range[2];
@@ -1376,8 +1381,10 @@ static errcode_t unix_discard(io_channel channel, unsigned long long block,
#endif
}
if (ret < 0) {
- if (errno == EOPNOTSUPP)
+ if (errno == EOPNOTSUPP) {
+ channel->flags |= CHANNEL_FLAGS_NODISCARD;
goto unimplemented;
+ }
return errno;
}
return 0;
@@ -1425,9 +1432,6 @@ static errcode_t unix_zeroout(io_channel channel, unsigned long long block,
data = (struct unix_private_data *) channel->private_data;
EXT2_CHECK_MAGIC(data, EXT2_ET_MAGIC_UNIX_IO_CHANNEL);

- if (safe_getenv("UNIX_IO_NOZEROOUT"))
- goto unimplemented;
-
if (!(channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE)) {
/* Regular file, try to use truncate/punch/zero. */
struct stat statbuf;
@@ -1450,13 +1454,18 @@ static errcode_t unix_zeroout(io_channel channel, unsigned long long block,
}
}

+ if (channel->flags & CHANNEL_FLAGS_NOZEROOUT)
+ goto unimplemented;
+
ret = __unix_zeroout(data->dev,
(off_t)(block) * channel->block_size + data->offset,
(off_t)(count) * channel->block_size);
err:
if (ret < 0) {
- if (errno == EOPNOTSUPP)
+ if (errno == EOPNOTSUPP) {
+ channel->flags |= CHANNEL_FLAGS_NOZEROOUT;
goto unimplemented;
+ }
return errno;
}
return 0;
--
1.8.3.1


2023-12-07 16:06:16

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext2fs: don't retry discard/zeroout repeatedly


On Tue, 09 May 2023 23:18:49 -0600, Andreas Dilger wrote:
> Call safe_getenv(UNIX_IO_NOZEROOUT) once when the device is
> opened and set CHANNEL_FLAG_NOZEROOUT if present instead of
> getting uid/euid/getenv every time unix_zeroout() is called.
>
> For unix_discard() and unix_zeroout() don't continue to call
> them if the block device doesn't support these operations.
>
> [...]

Applied, thanks!

[1/1] ext2fs: don't retry discard/zeroout repeatedly
commit: cc20e3c4320ae34dd06ec4d6a71d07aa7d6599d7

Best regards,
--
Theodore Ts'o <[email protected]>