2016-08-05 // Debugging Segfaults in Open-iSCSIs iscsiuio on Intel Broadwell
Open-iSCSIs tool iscsiuio, which is used to configure and manage Broadcom BCM577xx and BCM578xx iSCSI offload engines (iSOE), currently crashes with a segmentation fault upon target login on Intel Broadwell based systems. Comparable system setups, based on the older Intel Haswell chips do not show this issue.
In the past i've been using QLogic 4000 and QLogic 8200 Series network adapters and iSCSI HBAs which provide a full iSCSI offload engine (iSOE) implementation in the adapters firmware. Unfortunately the QLogic 8200 Series network adapters are no longer available for Dell M-Series blade servers. The alternatives offered by Dell are the Intel X520 and Intel X540 series adapters, or the Broadcom BCM57810S series adapters. Instead of using the Intel X520/X540, which provide no iSOE at all, i decided to go with the Broadcom BCM57810S, which at least provide some kind of iSOE. According to the VMware terminology the Broadcom BCM57810S are dependent hardware iSCSI Initiators. Dependent in this context means, that the iSOE does not implement all the necessary features and thus cannot perform all the tasks (e.g. TCP/IP stack, configuration and session management, authentication, etc.) necessary for target handling by itself. Rather, some of these tasks are provided by a third party on which this kind of iSOE depends on. In case of the Broadcom BCM57810S this third party is the iscsiuio daemon, which has for some time been part of the Open-iSCSI project. Simply put, the iscsiuio daemon acts as an intermediary between the iscsid on the one side and the QLogic1) NetXtreme II driver (kernel module bnx2 or bnx2x) and the QLogic2) CNIC driver (kernel module cnic) on the other side, facilitating the creation and overall management of offloaded iSCSI sessions. Very simplified, the flow of information is as follows:
iscsiadm ←→ iscsid ←→ iscsiuio ←→ bnx2/bnx2x ←→ cnic ←→ Broadcom BCM57810S adapter ←→ Network ←→ Target
In my environment the Broadcom BCM57810S adapters are installed and used on six hosts (host1 and host{5,6,7,8,9}). They all connect to the same Dell EqualLogic storage systems in the backend, using the same network dedicated to iSCSI traffic. All hosts are Dell M630 blade servers with exactly the same firmware, operating system (Debian 8) and software versions. I'm using a backported version of Open-iSCSI, which is based on Git commit 0fa43f29, but excluding the commits 76832662 and c6d1117b which just implement the externalization of the Open-iSNS code. The systems originate from the same install image, so their configuration is – to a large extent – exactly the same. The only difference between the hosts is that host1 has Intel E5 v3 (aka Haswell) CPUs, while host{5,6,7,8,9} have Intel E5 v4 (aka Broadwell) CPUs.
On host1 everything works fine, iscsiuio runs as expected and access to targets via the Broadcom BCM57810S iSOEs is working flawlessly. On host{5,6,7,8,9} on the other hand, i was getting segmentation faults like the one in the example shown below, while trying to log in to any target.
host5:~# gdb /sbin/iscsiuio GNU gdb (Debian 7.7.1+dfsg-5) 7.7.1 Copyright (C) 2014 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from /sbin/iscsiuio...(no debugging symbols found)...done. (gdb) # run -d 4 -f Starting program: /sbin/iscsiuio -d 4 -f [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". INFO [Wed Jul 27 10:01:45 2016]Initialize logger using log file: /var/log/iscsiuio.log INFO [Wed Jul 27 10:01:45 2016]Started iSCSI uio stack: Ver 0.7.8.2 INFO [Wed Jul 27 10:01:45 2016]Build date: Fri Jul 22 15:40:04 CEST 2016 INFO [Wed Jul 27 10:01:45 2016]Debug mode enabled INFO [Wed Jul 27 10:01:45 2016]Running on sysname: 'Linux', release: '3.16.0-4-amd64', version '#1 SMP Debian 3.16.7-ckt 25-2+deb8u3 (2016-07-02)' machine: 'x86_64' DBG [Wed Jul 27 10:01:45 2016]Loaded nic library 'bnx2' Version: '0.7.8.2' build on Fri Jul 22 15:40:04 CEST 2016' DBG [Wed Jul 27 10:01:45 2016]Added 'bnx2' nic library DBG [Wed Jul 27 10:01:45 2016]Loaded nic library 'bnx2x' Version: '0.7.8.2' build on Fri Jul 22 15:40:04 CEST 2016' DBG [Wed Jul 27 10:01:45 2016]Added 'bnx2x' nic library [New Thread 0x7ffff760f700 (LWP 4942)] INFO [Wed Jul 27 10:01:45 2016]signal handling thread ready INFO [Wed Jul 27 10:01:45 2016]nic_utils Found host[11]: host11 INFO [Wed Jul 27 10:01:45 2016]Done capturing /sys/class/iscsi_host/host11/netdev INFO [Wed Jul 27 10:01:45 2016]Done capturing /sys/class/iscsi_host/host11/netdev INFO [Wed Jul 27 10:01:45 2016]nic_utils looking for uio device for eth3 WARN [Wed Jul 27 10:01:45 2016]Could not find assoicate uio device with eth3 ERR [Wed Jul 27 10:01:45 2016]nic_utils Could not determine UIO name for eth3 INFO [Wed Jul 27 10:01:45 2016]nic_utils Found host[12]: host12 INFO [Wed Jul 27 10:01:45 2016]Done capturing /sys/class/iscsi_host/host12/netdev INFO [Wed Jul 27 10:01:45 2016]Done capturing /sys/class/iscsi_host/host12/netdev INFO [Wed Jul 27 10:01:45 2016]nic_utils looking for uio device for eth2 INFO [Wed Jul 27 10:01:45 2016]nic_utils eth2 associated with uio0 INFO [Wed Jul 27 10:01:45 2016]nic_utils NIC not found creating an instance for host_no: 12 eth2 DBG [Wed Jul 27 10:01:45 2016]Could not increase process priority: Success [New Thread 0x7ffff6e0e700 (LWP 4943)] DBG [Wed Jul 27 10:01:45 2016]iscsi_ipc Started iscsid listening thread DBG [Wed Jul 27 10:01:45 2016]iscsi_ipc Waiting for iscsid command INFO [Wed Jul 27 10:01:45 2016]NIC_NL Netlink to CNIC on pid 4938 is ready DBG [Wed Jul 27 10:01:57 2016]iscsi_ipc recv iscsid request: cmd: 1, payload_len: 11720 INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc Received request for 'eth2' to set IP address: '10.0.1.62' VLAN: '0' INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc Using netmask: 0.0.0.0 INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc eth2, using existing NIC INFO [Wed Jul 27 10:01:57 2016]nic_utils looking for uio device for eth2 INFO [Wed Jul 27 10:01:57 2016]nic_utils eth2 associated with uio0 INFO [Wed Jul 27 10:01:57 2016]Done capturing /sys/class/uio/uio0/name INFO [Wed Jul 27 10:01:57 2016]nic_utils eth2: Verified uio name bnx2x_cnic with library bnx2x INFO [Wed Jul 27 10:01:57 2016]eth2: found NIC with library 'bnx2x' INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc eth2 library set using transport_name bnx2i INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc eth2: requesting configuration using static IP address DBG [Wed Jul 27 10:01:57 2016]iscsi_ipc eth2 couldn't find interface with ip_type: 0x2 creating it INFO [Wed Jul 27 10:01:57 2016]nic eth2: Added nic interface for VLAN: 0, protocol: 2 INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc eth2: created network interface [New Thread 0x7ffff660d700 (LWP 4947)] WARN [Wed Jul 27 10:01:57 2016]nic_utils eth2: device already disabled: flag: 0x1088 state: 0x1 INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc eth2: configuring using static IP IPv4 address :10.0.1.62 INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc netmask: 255.255.255.0 [New Thread 0x7ffff5e0c700 (LWP 4948)] INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc ISCSID_UIP_IPC_GET_IFACE: command: 1 name: bnx2i.d0:43:1e:51:98:53, netdev: eth2 ipaddr: 10.0.1.62 vlan: 0 transport_name:bnx2i INFO [Wed Jul 27 10:01:57 2016]nic_utils eth2: spinning up thread for nic DBG [Wed Jul 27 10:01:57 2016]iscsi_ipc Waiting for iscsid command [New Thread 0x7ffff560b700 (LWP 4949)] DBG [Wed Jul 27 10:01:57 2016]nic eth2: Waiting to be enabled INFO [Wed Jul 27 10:01:57 2016]Created nic thread: eth2 INFO [Wed Jul 27 10:01:57 2016]iscsi_ipc eth2: started NIC enable thread state: 0x1 DBG [Wed Jul 27 10:01:57 2016]nic eth2: is now enabled INFO [Wed Jul 27 10:01:57 2016]bnx2x eth2: bnx2x driver using version 1.78.19 ERR [Wed Jul 27 10:01:58 2016]bnx2x /dev/uio0: uio device has been brought up via pid: 4938 on fd: 7 INFO [Wed Jul 27 10:01:58 2016]Done capturing /sys/class/uio/uio0/name INFO [Wed Jul 27 10:01:58 2016]bnx2x eth2: Verified is a cnic_uio device DBG [Wed Jul 27 10:01:58 2016]bnx2x eth2: using rx ring size: 15, rx buffer size: 1024 INFO [Wed Jul 27 10:01:58 2016]Done capturing /sys/class/uio/uio0/event DBG [Wed Jul 27 10:01:58 2016]bnx2x Chip ID: 168e1000 INFO [Wed Jul 27 10:01:58 2016]nic_id eth2: is found at 03:00.00 INFO [Wed Jul 27 10:01:58 2016]bnx2x eth2: func 0x0, pfid 0x0, client_id 0x88, cid 0x1 DBG [Wed Jul 27 10:01:58 2016]bnx2x eth2: mode = 0x100 INFO [Wed Jul 27 10:01:58 2016]bnx2x eth2: Using mac address: d0:43:1e:51:98:53 INFO [Wed Jul 27 10:01:58 2016]eth2: bnx2x initialized INFO [Wed Jul 27 10:01:58 2016]nic eth2: Initialized ip stack: VLAN: 0 INFO [Wed Jul 27 10:01:58 2016]nic eth2: mac: d0:43:1e:51:98:53 INFO [Wed Jul 27 10:01:58 2016]nic eth2: Using IP address: 10.0.1.62 INFO [Wed Jul 27 10:01:58 2016]nic eth2: Using netmask: 255.255.255.0 INFO [Wed Jul 27 10:01:58 2016]nic eth2: enabled vlan 0 protocol: 2 INFO [Wed Jul 27 10:01:58 2016]nic eth2: entering main nic loop DBG [Wed Jul 27 10:01:58 2016]nic_utils eth2: device enabled [Thread 0x7ffff5e0c700 (LWP 4948) exited] DBG [Wed Jul 27 10:01:59 2016]iscsi_ipc recv iscsid request: cmd: 1, payload_len: 11720 INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc Received request for 'eth2' to set IP address: '10.0.1.62' VLAN: '0' INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc Using netmask: 0.0.0.0 INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc eth2, using existing NIC INFO [Wed Jul 27 10:01:59 2016]nic_utils looking for uio device for eth2 INFO [Wed Jul 27 10:01:59 2016]nic_utils eth2 associated with uio0 INFO [Wed Jul 27 10:01:59 2016]eth2: Have NIC library 'bnx2x' INFO [Wed Jul 27 10:01:59 2016]Done capturing /sys/class/uio/uio0/name INFO [Wed Jul 27 10:01:59 2016]nic_utils eth2: Verified uio name bnx2x_cnic with library bnx2x INFO [Wed Jul 27 10:01:59 2016]eth2: found NIC with library 'bnx2x' INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc eth2 library set using transport_name bnx2i INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc eth2: requesting configuration using static IP address INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc eth2: using existing network interface INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc eth2: IP configuration didn't change using 0x2 INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc eth2: NIC already enabled flags: 0x1084 state: 0x4 INFO [Wed Jul 27 10:01:59 2016]iscsi_ipc ISCSID_UIP_IPC_GET_IFACE: command: 1 name: bnx2i.d0:43:1e:51:98:53, netdev: eth2 ipaddr: 10.0.1.62 vlan: 0 transport_name:bnx2i DBG [Wed Jul 27 10:01:59 2016]iscsi_ipc Waiting for iscsid command INFO [Wed Jul 27 10:02:00 2016]NIC_NL Received path_req for host 12 INFO [Wed Jul 27 10:02:00 2016]Done capturing /sys/class/iscsi_host/host12/netdev DBG [Wed Jul 27 10:02:00 2016]NIC_NL Pulled nl event INFO [Wed Jul 27 10:02:00 2016]NIC_NL eth2: Processing 'path_req' DBG [Wed Jul 27 10:02:00 2016]NIC_NL eth2: PATH_REQ with iface_num -1 VLAN 32768 DBG [Wed Jul 27 10:02:00 2016]CNIC eth2: Netlink message with VLAN ID: 0, path MTU: 9000 minor: 0 ip_addr_len: 4 DBG [Wed Jul 27 10:02:00 2016]CNIC eth2: src=10.0.1.62 DBG [Wed Jul 27 10:02:00 2016]CNIC eth2: dst=10.0.1.2 DBG [Wed Jul 27 10:02:00 2016]CNIC eth2: nm=255.255.255.0 INFO [Wed Jul 27 10:02:00 2016]CNIC eth2: Didn't find IPv4: '10.0.1.2' in ARP table DBG [Wed Jul 27 10:02:00 2016]CNIC eth2: Sent cnic arp request for IP: 10.0.1.2 INFO [Wed Jul 27 10:02:00 2016]Found 10.0.1.2 at b0:83:fe:cc:57:bb DBG [Wed Jul 27 10:02:00 2016]CNIC neighbor reply sent back to kernel 10.0.1.62 at b0:83:fe:cc:57:bb with vlan 0 INFO [Wed Jul 27 10:02:00 2016]NIC_NL eth2: 'path_req' operation finished Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff660d700 (LWP 4947)] __lll_unlock_elision (lock=0x55555577fd40, private=0) at ../nptl/sysdeps/unix/sysv/linux/x86/elision-unlock.c:29 29 ../nptl/sysdeps/unix/sysv/linux/x86/elision-unlock.c: No such file or directory. (gdb) # info threads Id Target Id Frame 6 Thread 0x7ffff560b700 (LWP 4949) "iscsiuio" 0x00007ffff76f1ae3 in select () at ../sysdeps/unix/syscall-template.S:81 * 4 Thread 0x7ffff660d700 (LWP 4947) "iscsiuio" __lll_unlock_elision (lock=0x55555577fd40, private=0) at ../nptl/sysdeps/unix/sysv/linux/x86/elision-unlock.c:29 3 Thread 0x7ffff6e0e700 (LWP 4943) "iscsiuio" 0x00007ffff79c9ccd in accept () at ../sysdeps/unix/syscall-template.S:81 2 Thread 0x7ffff760f700 (LWP 4942) "iscsiuio" do_sigwait (set=<optimized out>, sig=0x7ffff760eeac) at ../nptl/sysdeps/unix/sysv/linux/../../../../../sysdeps/unix/sysv/linux/sigwait.c:63 1 Thread 0x7ffff7fea700 (LWP 4938) "iscsiuio" 0x00007ffff79c9e9d in recvmsg () at ../sysdeps/unix/syscall-template.S:81 (gdb) # thread apply all bt Thread 6 (Thread 0x7ffff560b700 (LWP 4949)): #0 0x00007ffff76f1ae3 in select () at ../sysdeps/unix/syscall-template.S:81 #1 0x000055555555ac06 in ?? () #2 0x000055555555d39e in nic_loop () #3 0x00007ffff79c30a4 in start_thread (arg=0x7ffff560b700) at pthread_create.c:309 #4 0x00007ffff76f887d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 Thread 4 (Thread 0x7ffff660d700 (LWP 4947)): #0 __lll_unlock_elision (lock=0x55555577fd40, private=0) at ../nptl/sysdeps/unix/sysv/linux/x86/elision-unlock.c:29 #1 0x00007ffff79c7007 in pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:94 #2 0x000055555555e803 in nl_process_handle_thread () #3 0x00007ffff79c30a4 in start_thread (arg=0x7ffff660d700) at pthread_create.c:309 #4 0x00007ffff76f887d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 Thread 3 (Thread 0x7ffff6e0e700 (LWP 4943)): #0 0x00007ffff79c9ccd in accept () at ../sysdeps/unix/syscall-template.S:81 #1 0x00005555555641b0 in ?? () #2 0x00007ffff79c30a4 in start_thread (arg=0x7ffff6e0e700) at pthread_create.c:309 #3 0x00007ffff76f887d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 Thread 2 (Thread 0x7ffff760f700 (LWP 4942)): #0 do_sigwait (set=<optimized out>, sig=0x7ffff760eeac) at ../nptl/sysdeps/unix/sysv/linux/../../../../../sysdeps/unix/sysv/linux/sigwait.c:63 #1 0x00007ffff79ca693 in __sigwait (set=0x7ffff760eeb0, sig=0x0) at ../nptl/sysdeps/unix/sysv/linux/../../../../../sysdeps/unix/sysv/linux/sigwait.c:97 #2 0x000055555555a49c in _start () Thread 1 (Thread 0x7ffff7fea700 (LWP 4938)): #0 0x00007ffff79c9e9d in recvmsg () at ../sysdeps/unix/syscall-template.S:81 #1 0x000055555555e5e9 in ?? () #2 0x000055555555eea8 in nic_nl_open () #3 0x000055555555a1b8 in main ()
The Open-iSCSI command which lead to this segmentation fault was a simple login at a previously defined target node:
host5:~# iscsiadm -m node -T <target iqn> -I <interface> --login
Double-checking the configuration, the firmware and software versions as well as the general hardware setup didn't yield any usable indication as to where the root cause of this issue might be. Searching the web for __lll_unlock_elision in conjunction with the pthread_* function calls, led me to the following resources:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=800574
https://lwn.net/Articles/534758/
Those are pointing towards a CPU (Broadwell and Skylake) specific problem when not carefully using mutexes. The general opinion from there and also other related bug reports seems to be, that the source of such issues is almost always an improper use of mutex locking, which – up to now – has either been tolerated or just by chance not lead to a failure. More recent CPUs and software implementations (e.g. the GNU libc) appear to be less forgiving in this regard. Thus the advice is to change the application behaviour towards a proper use of mutex locking, in order to address such an issue.
The article Intel's Broadwell Xeon E5-2600 v4 chips: So what's in it for you, smartie-pants coders offers a rather nice write-up of the new features introduced in the Intel Broadwell CPUs.
Tracking this issue further down in the Open-iSCSI sources, i ended up in the function nl_process_handle_thread() in iscsiuio/src/unix/nic_nl.c and specifically in the following code section:
- iscsiuio/src/unix/nic_nl.c
474 /* NIC specific nl processing thread */ 475 void *nl_process_handle_thread(void *arg) 476 { [...] 483 while (!event_loop_stop) { 484 char *data = NULL; 485 486 rc = pthread_cond_wait(&nic->nl_process_cond, 487 &nic->nl_process_mutex); 488 if (rc != 0) { 489 LOG_ERR("Fatal error in NL processing thread " 490 "during wait[%s]", strerror(rc)); 491 break; 492 } [...] 499 pthread_mutex_unlock(&nic->nl_process_mutex); [...]
Debugging this revealed that the call to pthread_cond_wait() from the above GDB backtrace output of thread number 4 is the one from line 486 in the above code snippet.
Looking at the pthread_cond_wait() manpage showed the following constraint for its proper use:
[…]
The pthread_cond_timedwait() and pthread_cond_wait() functions shall
block on a condition variable. They shall be called with mutex locked
by the calling thread or undefined behavior results.
[…]
Although not shown in the above GDB output, this would on occasion – and again, probably just by chance – work on the first pass of the loop. At the end of the loop, at line 499 in the above code snippet, the mutex is then unlocked. Thus the cited constraint from the pthread_cond_wait() manpage is no longer met on the subsequent passes of the loop. On Intel E5 v3 (aka Haswell) CPUs, this seemed to be tolerated and without any impact. But on Intel E5 v4 (aka Broadwell) – and probably other CPUs implementing HLE and RTM – this causes the observed segmentation fault.
In order to verify my analysis and test this theory, i added a call to pthread_mutex_lock() right before the call to pthread_cond_wait() in line 486. The resulting change is available in the Git commit 9f770f9e of my Open-iSCSI fork on Github and also shown in the following patch:
- nic_nl.c.patch
diff --git a/iscsiuio/src/unix/nic_nl.c b/iscsiuio/src/unix/nic_nl.c index 391003f..581ddb0 100644 --- a/iscsiuio/src/unix/nic_nl.c +++ b/iscsiuio/src/unix/nic_nl.c @@ -483,6 +483,7 @@ void *nl_process_handle_thread(void *arg) while (!event_loop_stop) { char *data = NULL; + pthread_mutex_lock(&nic->nl_process_mutex); rc = pthread_cond_wait(&nic->nl_process_cond, &nic->nl_process_mutex); if (rc != 0) {
Posting this on the Open-iSCSI mailing list lead to this discussion. The suggestion from there was an additional change to the error handling code of nl_process_handle_thread() starting at line 488 in the above above code snippet. This adds proper handling of the locked mutex in case the loop is left due to an error returned from the call to pthread_cond_wait(). The resulting additional change is available in the Git commit 4191ca6b of my Open-iSCSI fork on Github. The following patch shows the summarised code changes:
- nic_nl.c.patch
diff --git a/iscsiuio/src/unix/nic_nl.c b/iscsiuio/src/unix/nic_nl.c index 391003f..1a920c7 100644 --- a/iscsiuio/src/unix/nic_nl.c +++ b/iscsiuio/src/unix/nic_nl.c @@ -483,9 +483,11 @@ void *nl_process_handle_thread(void *arg) while (!event_loop_stop) { char *data = NULL; + pthread_mutex_lock(&nic->nl_process_mutex); rc = pthread_cond_wait(&nic->nl_process_cond, &nic->nl_process_mutex); if (rc != 0) { + pthread_mutex_unlock(&nic->nl_process_mutex); LOG_ERR("Fatal error in NL processing thread " "during wait[%s]", strerror(rc)); break;
With those two small changes to the sources of Open-iSCSIs iscsiuio, the iSCSI connections via the Broadcom BCM57810S iSOE do now work flawlessly even on newer Intel E5 v4 (aka Broadwell) based systems. Hopefully the original authors of the iscsiuio code at Broadcom/QLogic will also take part in the discussion and provide their feedback on the proposed code changes too.
