On 12/04/2012 05:44 AM, Nicholas A. Bellinger wrote:
Hi Sebastian,
Hi Nab,
vfs_readv() returned -14 for non S_ISBLK
Ok, this looks like a perfectly normal READ_10 of 64K (128 sectors) to
LBA 12349.
Not sure why vfs_readv() of 64K is generating an -EFAULT here, but lets
first verify that SGLs being dispatched by tcm_loop to TCM -> FILEIO are
looking as expected on your setup.
Please try to reproduce with the following patch:
I modified it slightly in order to dump sg_virt() as well since this is
what we pass to the function that fails:
vfs_readv() returned -14 for non S_ISBLK
SG[0]: f4188240 page: f5c1e5a0 virt: (null) length: 4096 offset: 0
SG[1]: f4188258 page: f5c55da0 virt: (null) length: 4096 offset: 0
SG[2]: f4188270 page: f5c54d80 virt: (null) length: 4096 offset: 0
SG[3]: f4188288 page: f5c5bd20 virt: (null) length: 4096 offset: 0
SG[4]: f41882a0 page: f5c5bca0 virt: (null) length: 4096 offset: 0
SG[5]: f41882b8 page: f5c547e0 virt: (null) length: 4096 offset: 0
SG[6]: f41882d0 page: f5c58b20 virt: (null) length: 4096 offset: 0
SG[7]: f41882e8 page: f5c621e0 virt: (null) length: 4096 offset: 0
SG[8]: f4188300 page: f5c54d40 virt: ff990000 length: 4096 offset: 0
SG[9]: f4188318 page: f5c4c800 virt: (null) length: 4096 offset: 0
SG[10]: f4188330 page: f5c54ca0 virt: (null) length: 4096 offset: 0
SG[11]: f4188348 page: f5c57980 virt: (null) length: 4096 offset: 0
SG[12]: f4188360 page: f5c544e0 virt: (null) length: 4096 offset: 0
SG[13]: f4188378 page: f5c565c0 virt: (null) length: 4096 offset: 0
SG[14]: f4188390 page: f5c5bce0 virt: ff995000 length: 4096 offset: 0
SG[15]: f41883a8 page: f5c22980 virt: (null) length: 4096 offset: 0
If you ask why is virt null then I got to tell you that I have highmem.
I remember that while I fixed tcm_loop a year ago or so I pointed out
that the code is not highmem safe but it is not a problem because all
memory that is allocated by tcm_loop itself is non-highmem. No you got
highmem from somewhere.
The patch
diff --git a/drivers/target/target_core_file.c
b/drivers/target/target_core_file.c
index 0360383..319b87a 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -260,7 +260,7 @@ static int fd_do_readv(struct se_cmd *cmd, struct
scatterlist *sgl,
for_each_sg(sgl, sg, sgl_nents, i) {
iov[i].iov_len = sg->length;
- iov[i].iov_base = sg_virt(sg);
+ iov[i].iov_base = kmap(sg_page(sg));
}
old_fs = get_fs();
@@ -268,6 +268,10 @@ static int fd_do_readv(struct se_cmd *cmd, struct
scatterlist *sgl,
ret = vfs_readv(fd, &iov[0], sgl_nents, &pos);
set_fs(old_fs);
+ for_each_sg(sgl, sg, sgl_nents, i) {
+ kunmap(sg_page(sg));
+ }
+
kfree(iov);
/*
* Return zeros and GOOD status even if the READ did not return
Does not work. It bails out even earlier, before the XFS mount
completes. That means the complete code is not highmem aware and it
simply works because page_address() returns something that does not
cause a fault and the memory is not important enough to crash the box.
commit 8f9f44f8957b262de717a48269a5ceca36c2b504
Author: Nicholas Bellinger<nab@xxxxxxxxxxxxxxx>
Date: Mon Oct 1 23:29:49 2012 -0700
tcm_loop: Convert I/O path to use target_submit_cmd_map_sgls
This patch converts tcm_loop to use target_submit_cmd_map_sgls() for
I/O submission and mapping of pre-allocated SGL memory from incoming
scsi_cmnd -> se_cmd descriptors.
This includes removing the original open-coded fabric uses of target
core callers to support transport_generic_map_mem_to_cmd() between
target_setup_cmd_from_cdb() and transport_handle_cdb_direct() logic.
(v2: Use renamed target_submit_cmd_map_sgls)
Reported-by: Christoph Hellwig<hch@xxxxxx>
Reviewed-by: Christoph Hellwig<hch@xxxxxx>
Signed-off-by: Nicholas Bellinger<nab@xxxxxxxxxxxxxxx>
Very strange. This is a non functional change for tcm_loop code using
the exact same underlying target_core_mod functions for se_cmd setup ->
SGL mapping -> se_cmd I/O dispatch ub target_submit_cmd_map_sg(), so I
don't see how it would effect this bug.
Is it possible that now the memory allocation works different? I
haven't look at the code…
Thanks for reporting!
--nab
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
