Re: [PATCH v3 1/7] file-posix: fix max_iov for /dev/sg devices
On 03.06.21 15:37, Paolo Bonzini wrote:
Even though it was only called for devices that have bs->sg set (which
must be character devices),
sg_get_max_segments looked at /sys/dev/block which only works for
block devices.
On Linux the sg driver has its own way to provide the maximum number of
iovecs in a scatter/gather list.
Signed-off-by: Paolo Bonzini
---
block/file-posix.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index f37dfc10b3..58db526cc2 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1180,6 +1180,17 @@ static int sg_get_max_segments(int fd)
goto out;
}
+if (S_ISCHR(st->st_mode)) {
+if (ioctl(fd, SG_GET_SG_TABLESIZE, ) == 0) {
+return ret;
+}
+return -EIO;
+}
+
+if (!S_ISBLK(st->st_mode)) {
+return -ENOTSUP;
+}
+
With %s/->/./:
Reviewed-by: Max Reitz
(To answer Vladimir’s question, I don’t believe the condition should be
bs->sg, because bs->sg == true is a given for this function anyway.
Therefore, there’s no need to check whether the char device is an SG
device.)
Re: [PATCH v3 1/7] file-posix: fix max_iov for /dev/sg devices
03.06.2021 16:37, Paolo Bonzini wrote:
Even though it was only called for devices that have bs->sg set (which
must be character devices),
sg_get_max_segments looked at /sys/dev/block which only works for
block devices.
I assume, you keep /sys/dev/block code branch here only for following
converting of the function to be hdev_get_max_hw_transfer()? Worth mentioning
here?
On Linux the sg driver has its own way to provide the maximum number of
iovecs in a scatter/gather list.
Signed-off-by: Paolo Bonzini
---
block/file-posix.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index f37dfc10b3..58db526cc2 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1180,6 +1180,17 @@ static int sg_get_max_segments(int fd)
goto out;
}
+if (S_ISCHR(st->st_mode)) {
Should it be instead "if (bs->sg) {" ? As SG_GET_SG_TABLESIZE looks like
sg-specific, not for any chardev.
Also, st is not a pointer, so here and in the next if condition it should be
s/st->/st./
+if (ioctl(fd, SG_GET_SG_TABLESIZE, ) == 0) {
+return ret;
+}
+return -EIO;
+}
+
+if (!S_ISBLK(st->st_mode)) {
+return -ENOTSUP;
+}
+
sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
major(st.st_rdev), minor(st.st_rdev));
sysfd = open(sysfspath, O_RDONLY);
--
Best regards,
Vladimir
Re: [PATCH v3 1/7] file-posix: fix max_iov for /dev/sg devices
07.06.2021 08:39, Vladimir Sementsov-Ogievskiy wrote:
03.06.2021 16:37, Paolo Bonzini wrote:
Even though it was only called for devices that have bs->sg set (which
must be character devices),
sg_get_max_segments looked at /sys/dev/block which only works for
block devices.
On Linux the sg driver has its own way to provide the maximum number of
iovecs in a scatter/gather list.
Signed-off-by: Paolo Bonzini
---
block/file-posix.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index f37dfc10b3..58db526cc2 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1180,6 +1180,17 @@ static int sg_get_max_segments(int fd)
goto out;
}
+ if (S_ISCHR(st->st_mode)) {
+ if (ioctl(fd, SG_GET_SG_TABLESIZE, ) == 0) {
+ return ret;
So, this is a new most-probable path for char-dev, yes?
Is it better do it at start of the function to avoid extra fstat() ?
Haha, stupid question, sorry my morning sleepy eyes. fstat is used for outer if
condition.
+ }
+ return -EIO;
+ }
+
+ if (!S_ISBLK(st->st_mode)) {
+ return -ENOTSUP;
+ }
+
sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
major(st.st_rdev), minor(st.st_rdev));
sysfd = open(sysfspath, O_RDONLY);
--
Best regards,
Vladimir
Re: [PATCH v3 1/7] file-posix: fix max_iov for /dev/sg devices
03.06.2021 16:37, Paolo Bonzini wrote:
Even though it was only called for devices that have bs->sg set (which
must be character devices),
sg_get_max_segments looked at /sys/dev/block which only works for
block devices.
On Linux the sg driver has its own way to provide the maximum number of
iovecs in a scatter/gather list.
Signed-off-by: Paolo Bonzini
---
block/file-posix.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index f37dfc10b3..58db526cc2 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1180,6 +1180,17 @@ static int sg_get_max_segments(int fd)
goto out;
}
+if (S_ISCHR(st->st_mode)) {
+if (ioctl(fd, SG_GET_SG_TABLESIZE, ) == 0) {
+return ret;
So, this is a new most-probable path for char-dev, yes?
Is it better do it at start of the function to avoid extra fstat() ?
+}
+return -EIO;
+}
+
+if (!S_ISBLK(st->st_mode)) {
+return -ENOTSUP;
+}
+
sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
major(st.st_rdev), minor(st.st_rdev));
sysfd = open(sysfspath, O_RDONLY);
--
Best regards,
Vladimir
