Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed missing data type cast which was generating a warning/error. #24

Merged
merged 5 commits into from
Mar 10, 2021

Conversation

dwoolworth
Copy link
Contributor

While I'm not sure if this matters to anyone, I was attempting to get the iSCSI driver to compile and ran into this issue. Not sure if this is the appropriate place to submit, but at least if someone searches for anything like "iscsid: can not create NETLINK_ISCSI socket" and cannot figure out why iscsid is hanging, and modprobe iscsi_tcp says the module doesn't exist... Or while trying to compile a new kernel for the Jetson Nano to include the iscsi_tcp kernel driver, etc. and runs across this error, hopefully they'll find this post. Or maybe it'll just be me the next time I forget what I did to get this working.

@madisongh
Copy link
Member

This appears to be needed due to changes made in 28ef6f8. I'm not exactly sure why that change was made, and it doesn't appear upstream, so we may run into similar issues with any other driver that uses scatterlists.

@madisongh
Copy link
Member

28ef6f8 was submitted upstream, but it doesn't look like it went anywhere. It might be worth experimenting with just reverting it.

@dwoolworth
Copy link
Contributor Author

dwoolworth commented Mar 9, 2021

@madisongh hi, and thanks for the feedback. At first I was trying to revert that hash and recompile and then ran into quite a few other issues. Most likely I'm missing something in the build process. Nevertheless, going back to my original code base, I checked the scatterlist.h header and actually, the changes detailed in the hash's changes exist already.

struct scatterlist {
#ifdef CONFIG_DEBUG_SG
  unsigned long sg_magic;
#endif
  unsigned long page_link;
  unsigned long   offset;
  size_t    length;
  dma_addr_t  dma_address;
#ifdef CONFIG_NEED_SG_DMA_LENGTH
  size_t    dma_length;
#endif
};

My original code base comes from https://developer.nvidia.com/embedded/L4T/r32_Release_v4.4/r32_Release_v4.4-GMC3/Sources/T210/public_sources.tbz2

The drivers/scsi/libiscsi_tcp.c requires my changes, or at least the casting of size_t to unsigned int to make the min() comparison compile without a warning.

@madisongh
Copy link
Member

@dwoolworth If it's actually possible to have sg transfers with offsets or sizes > 4GiB, as implied by the NVIDIA changes to scatterlist.h, then wouldn't it be better to modify include/scsi/libiscsi_tcp.h to update the size and offset fields to be longs instead of ints (which would no doubt cause a cascade of other needed changes, ugh), or at least modify your changes to try and catch potential overflows? Just truncating the 64-bit values down to 32-bit doesn't sound like a good idea, unless you can be sure that such a situation would never actually occur.

@dwoolworth
Copy link
Contributor Author

dwoolworth commented Mar 10, 2021

@madisongh you are very obviously correct. I've updated the source accordingly. I would estimate there are quite a few scsi drivers left that need to be updated. I only fixed the iscsi_tcp related. I hope the updates are correct/useful. I have tested these as follows:

root@testbox:~# iscsiadm -m discovery -t st -p 192.168.0.20
192.168.0.20:3260,1 iqn.2000-01.com.synology:sdinas02.Target-1.2b065a0e3c
[2603:8090:e02:3900:211:32ff:fee1:9f9d]:3260,1 iqn.2000-01.com.synology:sdinas02.Target-1.2b065a0e3c
[fe80::211:32ff:fee1:9f9d]:3260,1 iqn.2000-01.com.synology:sdinas02.Target-1.2b065a0e3c
root@testbox:~# iscsiadm -m node -p 192.168.0.20 -T iqn.2000-01.com.synology:sdinas02.Target-1.2b065a0e3c --login
Logging in to [iface: default, target: iqn.2000-01.com.synology:sdinas02.Target-1.2b065a0e3c, portal: 192.168.0.20,3260] (multiple)
Login to [iface: default, target: iqn.2000-01.com.synology:sdinas02.Target-1.2b065a0e3c, portal: 192.168.0.20,3260] successful.
root@testbox:~# mkdir /data
root@testbox:~# mount /dev/sda /data
root@testbox:~# ls -al /data
total 24
drwxr-xr-x  3 root root  4096 Mar  7 20:11 .
drwxr-xr-x 23 root root  4096 Mar 10 10:53 ..
drwx------  2 root root 16384 Mar  7 19:22 lost+found
root@testbox:~# dd if=/dev/zero of=/data/testfile-4GB.out bs=1048576 count=4097
4097+0 records in
4097+0 records out
4296015872 bytes (4.3 GB, 4.0 GiB) copied, 34.5509 s, 124 MB/s
root@testbox:~# ls -al
total 32
drwx------  6 root root 4096 Jan 28  2018 .
drwxr-xr-x 23 root root 4096 Mar 10 10:53 ..
-rw-r--r--  1 root root 3106 Apr  9  2018 .bashrc
drwx------  6 root root 4096 Jan 28  2018 .cache
drwx------  5 root root 4096 Jan 28  2018 .config
drwx------  3 root root 4096 Jan 28  2018 .gnupg
drwx------  3 root root 4096 Jan 28  2018 .local
-rw-r--r--  1 root root  148 Aug 17  2015 .profile
root@testbox:~# ls -al /data
total 4195356
drwxr-xr-x  3 root root       4096 Mar 10 10:55 .
drwxr-xr-x 23 root root       4096 Mar 10 10:53 ..
drwx------  2 root root      16384 Mar  7 19:22 lost+found
-rw-r--r--  1 root root 4296015872 Mar 10 10:55 testfile-4GB.out
root@testbox:~# cp /data/testfile-4GB.out .
root@testbox:~# ls -al
total 4195364
drwx------  6 root root       4096 Mar 10 10:56 .
drwxr-xr-x 23 root root       4096 Mar 10 10:53 ..
-rw-r--r--  1 root root       3106 Apr  9  2018 .bashrc
drwx------  6 root root       4096 Jan 28  2018 .cache
drwx------  5 root root       4096 Jan 28  2018 .config
drwx------  3 root root       4096 Jan 28  2018 .gnupg
drwx------  3 root root       4096 Jan 28  2018 .local
-rw-r--r--  1 root root        148 Aug 17  2015 .profile
-rw-r--r--  1 root root 4296015872 Mar 10 10:57 testfile-4GB.out
root@testbox:~# 

@madisongh madisongh merged commit 2814618 into OE4T:oe4t-patches-l4t-r32.4 Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants