On Dec 4, 2020, at 9:58 PM, Theodore Ts'o <tytso@xxxxxxx> wrote:
>
> Add initial implementation support for the unix_io manager.
> Applications which want to use threading should pass in
> IO_FLAG_THREADS when opening the channel. Channels which support
> threading (which as of this commit is unix_io and test_io if the
> backing io_manager supports threading) will set the
> CHANNEL_FLAGS_THREADS bit in io->flags. Library code or applications
> can test if threading is enabled by checking this flag.
>
> Applications using libext2fs can pass in EXT2_FLAG_THREADS to
> ext2fs_open() or ext2fs_open2() to request threading support.
>
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> ---
>
> diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
> index 628e60c39..9385487d9 100644
> --- a/lib/ext2fs/unix_io.c
> +++ b/lib/ext2fs/unix_io.c
> @@ -232,8 +287,10 @@ static errcode_t raw_read_blk(io_channel channel,
> bounce_read:
> while (size > 0) {
> + mutex_lock(data, BOUNCE_MTX);
> actual = read(data->dev, data->bounce, channel->block_size);
> if (actual != channel->block_size) {
> + mutex_unlock(data, BOUNCE_MTX);
> actual = really_read;
> buf -= really_read;
> size += really_read;
> @@ -246,6 +303,7 @@ bounce_read:
> really_read += actual;
> size -= actual;
> buf += actual;
> + mutex_unlock(data, BOUNCE_MTX);
> }
> return 0;
Do you know how often we get into the "bounce_read" IO path? It seems like
locking around the read would kill parallelism, but this code path also
looks like a fallback, but maybe 100% used for blocksize != PAGE_SIZE?
> @@ -341,11 +401,13 @@ static errcode_t raw_write_blk(io_channel channel,
> */
> bounce_write:
> while (size > 0) {
> + mutex_lock(data, BOUNCE_MTX);
> if (size < channel->block_size) {
> actual = read(data->dev, data->bounce,
> channel->block_size);
> if (actual != channel->block_size) {
> if (actual < 0) {
> + mutex_unlock(data, BOUNCE_MTX);
> retval = errno;
> goto error_out;
> }
> @@ -362,6 +424,7 @@ bounce_write:
> goto error_out;
> }
> actual = write(data->dev, data->bounce, channel->block_size);
> + mutex_unlock(data, BOUNCE_MTX);
> if (actual < 0) {
> retval = errno;
> goto error_out;
Ditto.
> @@ -703,6 +773,25 @@ static errcode_t unix_open_channel(const char *name, int fd,
> setrlimit(RLIMIT_FSIZE, &rlim);
> }
> }
> +#endif
> +#ifdef HAVE_PTHREAD
> + if (flags & IO_FLAG_THREADS) {
> + io->flags |= CHANNEL_FLAGS_THREADS;
> + retval = pthread_mutex_init(&data->cache_mutex, NULL);
> + if (retval)
> + goto cleanup;
> + retval = pthread_mutex_init(&data->bounce_mutex, NULL);
> + if (retval) {
> + pthread_mutex_destroy(&data->cache_mutex);
> + goto cleanup;
> + }
> + retval = pthread_mutex_init(&data->stats_mutex, NULL);
> + if (retval) {
> + pthread_mutex_destroy(&data->cache_mutex);
> + pthread_mutex_destroy(&data->bounce_mutex);
> + goto cleanup;
> + }
> + }
> #endif
At one point you talked about using dlopen() or similar to link in the
pthread library only if it is actually needed? Or is the linkage of
the pthread library avoided by these functions not being called unless
IO_FLAG_THREADS is set?
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
