Skip to content

Commit 8cb578b

Browse files
adam900710kdave
authored andcommitted
btrfs-progs: split btrfs_direct_pio() functions into read and write
It's not a common practice to use the same io function for both read and write (we have pread() and pwrite(), not pio()). Furthermore the original function has the following problems: - Not returning proper error number If we had ioctl/stat errors we just return 0 with errno set. Thus caller would treat it as a short read, not a proper error. - Unnecessary @ret_rw This is not that obvious if we have different handling for read and write, but if we split them it's super obvious we can reuse @ret. - No proper copy back for short read - Unable to constify the @buf pointer for write operation All those problems would be addressed in this patch. Signed-off-by: Qu Wenruo <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 5b2617c commit 8cb578b

File tree

2 files changed

+60
-29
lines changed

2 files changed

+60
-29
lines changed

common/device-utils.c

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -529,21 +529,18 @@ int device_get_rotational(const char *file)
529529
return (rotational == '0');
530530
}
531531

532-
ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset)
532+
ssize_t btrfs_direct_pread(int fd, void *buf, size_t count, off_t offset)
533533
{
534534
int alignment;
535535
size_t iosize;
536536
void *bounce_buf = NULL;
537537
struct stat stat_buf;
538538
unsigned long req;
539539
int ret;
540-
ssize_t ret_rw;
541-
542-
UASSERT(rw == READ || rw == WRITE);
543540

544541
if (fstat(fd, &stat_buf) == -1) {
545542
error("fstat failed: %m");
546-
return 0;
543+
return -errno;
547544
}
548545

549546
if ((stat_buf.st_mode & S_IFMT) == S_IFBLK)
@@ -553,20 +550,61 @@ ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset)
553550

554551
if (ioctl(fd, req, &alignment)) {
555552
error("failed to get block size: %m");
556-
return 0;
553+
return -errno;
557554
}
558555

559-
if (IS_ALIGNED((size_t)buf, alignment) && IS_ALIGNED(count, alignment)) {
560-
if (rw == WRITE)
561-
return pwrite(fd, buf, count, offset);
562-
else
563-
return pread(fd, buf, count, offset);
556+
if (IS_ALIGNED((size_t)buf, alignment) && IS_ALIGNED(count, alignment))
557+
return pread(fd, buf, count, offset);
558+
559+
iosize = round_up(count, alignment);
560+
561+
ret = posix_memalign(&bounce_buf, alignment, iosize);
562+
if (ret) {
563+
error_msg(ERROR_MSG_MEMORY, "bounce buffer");
564+
errno = ret;
565+
return -ret;
564566
}
565567

568+
ret = pread(fd, bounce_buf, iosize, offset);
569+
if (ret >= count)
570+
ret = count;
571+
memcpy(buf, bounce_buf, count);
572+
573+
free(bounce_buf);
574+
return ret;
575+
}
576+
577+
ssize_t btrfs_direct_pwrite(int fd, const void *buf, size_t count, off_t offset)
578+
{
579+
int alignment;
580+
size_t iosize;
581+
void *bounce_buf = NULL;
582+
struct stat stat_buf;
583+
unsigned long req;
584+
int ret;
585+
586+
if (fstat(fd, &stat_buf) == -1) {
587+
error("fstat failed: %m");
588+
return -errno;
589+
}
590+
591+
if ((stat_buf.st_mode & S_IFMT) == S_IFBLK)
592+
req = BLKSSZGET;
593+
else
594+
req = FIGETBSZ;
595+
596+
if (ioctl(fd, req, &alignment)) {
597+
error("failed to get block size: %m");
598+
return -errno;
599+
}
600+
601+
if (IS_ALIGNED((size_t)buf, alignment) && IS_ALIGNED(count, alignment))
602+
return pwrite(fd, buf, count, offset);
603+
566604
/* Cannot do anything if the write size is not aligned */
567-
if (rw == WRITE && !IS_ALIGNED(count, alignment)) {
605+
if (!IS_ALIGNED(count, alignment)) {
568606
error("%zu is not aligned to %d", count, alignment);
569-
return 0;
607+
return -EINVAL;
570608
}
571609

572610
iosize = round_up(count, alignment);
@@ -575,21 +613,13 @@ ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset)
575613
if (ret) {
576614
error_msg(ERROR_MSG_MEMORY, "bounce buffer");
577615
errno = ret;
578-
return 0;
616+
return -ret;
579617
}
580618

581-
if (rw == WRITE) {
582-
UASSERT(iosize == count);
583-
memcpy(bounce_buf, buf, count);
584-
ret_rw = pwrite(fd, bounce_buf, iosize, offset);
585-
} else {
586-
ret_rw = pread(fd, bounce_buf, iosize, offset);
587-
if (ret_rw >= count) {
588-
ret_rw = count;
589-
memcpy(buf, bounce_buf, count);
590-
}
591-
}
619+
UASSERT(iosize == count);
620+
memcpy(bounce_buf, buf, count);
621+
ret = pwrite(fd, bounce_buf, iosize, offset);
592622

593623
free(bounce_buf);
594-
return ret_rw;
624+
return ret;
595625
}

common/device-utils.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ int device_get_rotational(const char *file);
5050
*/
5151
int btrfs_prepare_device(int fd, const char *file, u64 *block_count_ret,
5252
u64 max_block_count, unsigned opflags);
53-
ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset);
53+
ssize_t btrfs_direct_pread(int fd, void *buf, size_t count, off_t offset);
54+
ssize_t btrfs_direct_pwrite(int fd, const void *buf, size_t count, off_t offset);
5455

5556
#ifdef BTRFS_ZONED
5657
static inline ssize_t btrfs_pwrite(int fd, void *buf, size_t count,
@@ -59,15 +60,15 @@ static inline ssize_t btrfs_pwrite(int fd, void *buf, size_t count,
5960
if (!direct)
6061
return pwrite(fd, buf, count, offset);
6162

62-
return btrfs_direct_pio(WRITE, fd, buf, count, offset);
63+
return btrfs_direct_pwrite(fd, buf, count, offset);
6364
}
6465
static inline ssize_t btrfs_pread(int fd, void *buf, size_t count, off_t offset,
6566
bool direct)
6667
{
6768
if (!direct)
6869
return pread(fd, buf, count, offset);
6970

70-
return btrfs_direct_pio(READ, fd, buf, count, offset);
71+
return btrfs_direct_pread(fd, buf, count, offset);
7172
}
7273
#else
7374
#define btrfs_pwrite(fd, buf, count, offset, direct) \

0 commit comments

Comments
 (0)