RE: [PATCH 19/55] scsi: Mark function as static in isci/phy.c
On Monday, March 31, 2014 11:36 AM Josh Triplett wrote: > On Mon, Mar 31, 2014 at 08:54:49AM +0000, Dorau, Lukasz wrote: > > On Saturday, March 29, 2014 7:04 PM Rashika Kheria > wrote: > > > > > > Mark function as static in isci/phy.c because it is not used outside > > > this file. > > > > > > This eliminates the following warning in isci/phy.c: > > > drivers/scsi/isci/phy.c:672:6: warning: no previous prototype for > > > ‘scu_link_layer_set_txcomsas_timeout’ [-Wmissing-prototypes] > > > > > > Signed-off-by: Rashika Kheria > > > Reviewed-by: Josh Triplett > > > > Acked-by: Lukasz Dorau > > Since you're a maintainer of the driver in question, can you accept the > three patches you acked, or would you suggest that they go through > another tree? > > - Josh Triplett Hi Josh, The isci patches should be accepted by James Bottomley. Lukasz
RE: [PATCH 20/55] scsi: Mark function as static in isci/remote_device.c
On Saturday, March 29, 2014 7:05 PM Rashika Kheria
wrote:
> Mark function as static in isci/remote_device.c because it is not used
> outside this file.
>
> This eliminates the following warning in isci/remote_device.c:
> drivers/scsi/isci/remote_device.c:1387:6: warning: no previous prototype for
> ‘isci_remote_device_wait_for_resume_from_abort’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
> Reviewed-by: Josh Triplett
Acked-by: Lukasz Dorau
> ---
> drivers/scsi/isci/remote_device.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/isci/remote_device.c
> b/drivers/scsi/isci/remote_device.c
> index 96a26f4..33033fb 100644
> --- a/drivers/scsi/isci/remote_device.c
> +++ b/drivers/scsi/isci/remote_device.c
> @@ -1384,7 +1384,7 @@ static bool isci_remote_device_test_resume_done(
> return done;
> }
>
> -void isci_remote_device_wait_for_resume_from_abort(
> +static void isci_remote_device_wait_for_resume_from_abort(
> struct isci_host *ihost,
> struct isci_remote_device *idev)
> {
> --
> 1.7.9.5
N�r��yb�X��ǧv�^�){.n�+{���"�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
RE: [PATCH 21/55] scsi: Mark function as static in isci/port.c
On Saturday, March 29, 2014 7:07 PM Rashika Kheria
wrote:
> Mark function as static in isci/port.c because they are not used outside
> this file.
>
> This eliminates the following warning in isci/port.c:
> drivers/scsi/isci/port.c:65:13: warning: no previous prototype for
> ‘port_state_name’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
> Reviewed-by: Josh Triplett
Acked-by: Lukasz Dorau
> ---
> drivers/scsi/isci/port.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
> index 13098b0..225d39f 100644
> --- a/drivers/scsi/isci/port.c
> +++ b/drivers/scsi/isci/port.c
> @@ -62,7 +62,7 @@
>
> #undef C
> #define C(a) (#a)
> -const char *port_state_name(enum sci_port_states state)
> +static const char *port_state_name(enum sci_port_states state)
> {
> static const char * const strings[] = PORT_STATES;
>
> --
> 1.7.9.5
RE: [PATCH 19/55] scsi: Mark function as static in isci/phy.c
On Saturday, March 29, 2014 7:04 PM Rashika Kheria
wrote:
>
> Mark function as static in isci/phy.c because it is not used outside
> this file.
>
> This eliminates the following warning in isci/phy.c:
> drivers/scsi/isci/phy.c:672:6: warning: no previous prototype for
> ‘scu_link_layer_set_txcomsas_timeout’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria
> Reviewed-by: Josh Triplett
Acked-by: Lukasz Dorau
> ---
> drivers/scsi/isci/phy.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/isci/phy.c b/drivers/scsi/isci/phy.c
> index cb87b2e..8e21022 100644
> --- a/drivers/scsi/isci/phy.c
> +++ b/drivers/scsi/isci/phy.c
> @@ -669,7 +669,8 @@ static const char *phy_event_name(u32 event_code)
> phy_state_name(state), phy_event_name(code), code)
>
>
> -void scu_link_layer_set_txcomsas_timeout(struct isci_phy *iphy, u32 timeout)
> +static void scu_link_layer_set_txcomsas_timeout(struct isci_phy *iphy,
> + u32 timeout)
> {
> u32 val;
>
> --
> 1.7.9.5
N�r��yb�X��ǧv�^�){.n�+{���"�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
RE: [PATCH v2 10/23] isci: Use pci_enable_msix_exact() instead of pci_enable_msix()
On Monday, February 24, 2014 9:02 AM Alexander Gordeev wrote: > As result of deprecation of MSI-X/MSI enablement functions > pci_enable_msix() and pci_enable_msi_block() all drivers > using these two interfaces need to be updated to use the > new pci_enable_msi_range() or pci_enable_msi_exact() > and pci_enable_msix_range() or pci_enable_msix_exact() > interfaces. > > Signed-off-by: Alexander Gordeev > Cc: Lukasz Dorau > Cc: Maciej Patelczyk > Cc: Dave Jiang > Cc: intel-linux-...@intel.com > Cc: linux-scsi@vger.kernel.org > Cc: linux-...@vger.kernel.org > --- > drivers/scsi/isci/init.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c > index d25d0d8..ce40538 100644 > --- a/drivers/scsi/isci/init.c > +++ b/drivers/scsi/isci/init.c > @@ -356,7 +356,7 @@ static int isci_setup_interrupts(struct pci_dev *pdev) > for (i = 0; i < num_msix; i++) > pci_info->msix_entries[i].entry = i; > > - err = pci_enable_msix(pdev, pci_info->msix_entries, num_msix); > + err = pci_enable_msix_exact(pdev, pci_info->msix_entries, num_msix); > if (err) > goto intx; > > -- > 1.7.7.6 Looks fine. Acked-by: Lukasz Dorau Thanks, Lukasz -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 10/22] isci: Use pci_enable_msix_range()
On Tuesday, February 04, 2014 12:17 PM Alexander Gordeev
wrote:
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers
> using these two interfaces need to be updated to use the
> new pci_enable_msi_range() and pci_enable_msix_range()
> interfaces.
>
> Signed-off-by: Alexander Gordeev
> Cc: Lukasz Dorau
> Cc: Maciej Patelczyk
> Cc: Dave Jiang
> Cc: intel-linux-...@intel.com
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> ---
> drivers/scsi/isci/init.c |5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> index d25d0d8..b99f307 100644
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -356,8 +356,9 @@ static int isci_setup_interrupts(struct pci_dev *pdev)
> for (i = 0; i < num_msix; i++)
> pci_info->msix_entries[i].entry = i;
>
> - err = pci_enable_msix(pdev, pci_info->msix_entries, num_msix);
> - if (err)
> + err = pci_enable_msix_range(pdev,
> + pci_info->msix_entries, num_msix, num_msix);
> + if (err < 0)
> goto intx;
>
> for (i = 0; i < num_msix; i++) {
> --
> 1.7.7.6
Looks fine.
Acked-by: Lukasz Dorau
Thanks,
Lukasz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] isci: update version to 1.2
On Thursday, January 23, 2014 10:39 AM Lukasz Dorau wrote: > The version of isci driver has not been updated for 2 years. > It was 83 isci commits ago. Suspend/resume support has been implemented > and many bugs have been fixed since 1.1. Now update the version to 1.2. > > Signed-off-by: Lukasz Dorau > Cc: Oops... By mistake I have sent the wrong version of the patch. I'm sorry. Please disregard it. Lukasz
RE: [PATCH] isci: correct erroneous for_each_isci_host macro
On Monday, January 20, 2014 7:26 PM Dan Williams wrote: > On Mon, Jan 20, 2014 at 8:54 AM, Lukasz Dorau wrote: > > In the first place, the loop 'for' in the macro 'for_each_isci_host' > > (drivers/scsi/isci/host.h:314) is incorrect, because it accesses > > the 3rd element of 2 element array. After the 2nd iteration it executes > > the instruction: > > ihost = to_pci_info(pdev)->hosts[2] > > (while the size of the 'hosts' array equals 2) and reads an > > out of range element. > > > > In the second place, because of the following GCC v4.8 bug: > > http://marc.info/?l=linux-kernel&m=138998871911336&w=2 > > this loop is incorrectly compiled by GCC v4.8. > > As a result, on platforms with two SCU controllers, > > the loop at drivers/scsi/isci/init.c:717 is executed > > more times than it can be (for i=0,1 and 2). > > It causes the following oops after 'rmmod isci': > > > > Surprised those bugs lasted this long... some comments below. > > > BUG: unable to handle kernel NULL pointer dereference at (null) > > IP: [] __list_add+0x1b/0xc0 > > Oops: [#1] SMP > > RIP: 0010:[] [] __list_add+0x1b/0xc0 > > Call Trace: > > [] __mutex_lock_slowpath+0x114/0x1b0 > > [] mutex_lock+0x1f/0x30 > > [] sas_disable_events+0x1b/0x50 [libsas] > > [] sas_unregister_ha+0x18/0x60 [libsas] > > [] isci_unregister+0x1e/0x40 [isci] > > [] isci_pci_remove+0x5d/0x100 [isci] > > [] pci_device_remove+0x3b/0xb0 > > [] __device_release_driver+0x7f/0xf0 > > [] driver_detach+0xa8/0xb0 > > [] bus_remove_driver+0x9b/0x120 > > [] driver_unregister+0x2c/0x50 > > [] pci_unregister_driver+0x23/0x80 > > [] isci_exit+0x10/0x1e [isci] > > [] SyS_delete_module+0x16b/0x2d0 > > [] ? do_notify_resume+0x61/0xa0 > > [] system_call_fastpath+0x16/0x1b > > > > The loop has been corrected. > > This patch fixes the above oops. > > > > Signed-off-by: Lukasz Dorau > > Reviewed-by: Maciej Patelczyk > > Tested-by: Lukasz Dorau > > Cc: Dan Williams > > Cc: Dave Jiang > > Cc: > > --- > > drivers/scsi/isci/host.h |6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h > > index 4911310..95adf31 100644 > > --- a/drivers/scsi/isci/host.h > > +++ b/drivers/scsi/isci/host.h > > @@ -311,9 +311,9 @@ static inline struct Scsi_Host *to_shost(struct > > isci_host > *ihost) > > } > > > > #define for_each_isci_host(id, ihost, pdev) \ > > - for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \ > > -id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \ > > -ihost = to_pci_info(pdev)->hosts[++id]) > > + for (id = 0, ihost = to_pci_info(pdev)->hosts[0]; \ > > No need to initialize ihost here since we assign and check it later. > > > +(id < ARRAY_SIZE(to_pci_info(pdev)->hosts)) && \ > > Just use SCI_MAX_CONTROLLERS here. > > > +(ihost = to_pci_info(pdev)->hosts[id]); id++) > > -- Thanks, Dan. I will submit the second version of the patch. Lukasz -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] isci: reformulate for_each_isci_host macro to fix an oops
On Thursday, January 16, 2014 2:16 PM Lukasz Dorau wrote: > > The loop 'for' in macro 'for_each_isci_host' (drivers/scsi/isci/init.c:717) > is executed more times than it can be. Regardless the condition: >'id < ARRAY_SIZE(to_pci_info(pdev)->hosts)' (drivers/scsi/isci/host.h:315) > it is executed when id equals ARRAY_SIZE(to_pci_info(pdev)->hosts) too. > (Remark: ARRAY_SIZE(to_pci_info(pdev)->hosts) always equals > SCI_MAX_CONTROLLERS = 2) > > It sounds crazy, but it is truth. I have checked it in the following way: > I have added the line: > >printk(KERN_ERR ">>> (%d < %d) == %d \n", \ > i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS)); > > after the 'for_each_isci_host' macro in drivers/scsi/isci/init.c:701 > and received the following output: > >>>> (0 < 2) == 1 >>>> (1 < 2) == 1 >>>> (2 < 2) == 1 > > after issuing 'modprobe isci' command on platform with two SCU controllers > (Patsburg D or T chipset required). > The kernel was compiled using gcc version 4.8.2. > Hi James, Please disregard this patch. It turned to be a GCC 4.8 bug: http://marc.info/?l=linux-kernel&m=138998871911336&w=2 I will write a new patch. Lukasz
RE: Why is (2 < 2) true? Is it a gcc bug?
On Friday, January 17, 2014 10:44 PM Alexei Starovoitov
wrote:
> On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
> wrote:
> > On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
> >> On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
> >> wrote:
> >> > On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz
> wrote:
> >> >> Hi
> >> >>
> >> >> My story is very simply...
> >> >> I applied the following patch:
> >> >>
> >> >> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> >> >> --- a/drivers/scsi/isci/init.c
> >> >> +++ b/drivers/scsi/isci/init.c
> >> >> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev,
> >> >> const
> struct pci_device_id *id)
> >> >> if (err)
> >> >> goto err_host_alloc;
> >> >>
> >> >> - for_each_isci_host(i, isci_host, pdev)
> >> >> + for_each_isci_host(i, isci_host, pdev) {
> >> >> + pr_err("(%d < %d) == %d\n",\
> >> >> + i, SCI_MAX_CONTROLLERS, (i <
> >> >> SCI_MAX_CONTROLLERS));
> >> >> scsi_scan_host(to_shost(isci_host));
> >> >> + }
> >> >>
> >> >> return 0;
> >> >>
> >> >> --
> >> >> 1.8.3.1
> >> >>
> >> >> Then I issued the command 'modprobe isci' on platform with two SCU
> controllers (Patsburg D or T chipset)
> >> >> and received the following, very strange, output:
> >> >>
> >> >> (0 < 2) == 1
> >> >> (1 < 2) == 1
> >> >> (2 < 2) == 1
> >> >>
> >> >> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
> >> >
> >> > gcc sees that i < array_size is the same as i < 2 as part of loop
> >> > condition, so
> >> > it optimizes (i < sci_max_controllers) into constant 1.
> >> > and emits printk like:
> >> > printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
> >> >
> >> >> (The kernel was compiled using gcc version 4.8.2.)
> >> >
> >> > it actually looks to be gcc 4.8 bug.
> >> > Can you try gcc 4.7 ?
> >> >
> >>
> >> It is interesting GCC 4.8 bug,
> >> since it seems to expose issues in two compiler passes.
> >>
> >> here is test case:
> >>
> >> struct isci_host;
> >> struct isci_orom;
> >>
> >> struct isci_pci_info {
> >> struct isci_host *hosts[2];
> >> struct isci_orom *orom;
> >> } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
> >>
> >> int printf(const char *fmt, ...);
> >>
> >> int isci_pci_probe()
> >> {
> >> int i;
> >> struct isci_host *isci_host;
> >>
> >> for (i = 0, isci_host = v.hosts[i];
> >>i < 2 && isci_host;
> >>isci_host = v.hosts[++i]) {
> >> printf("(%d < %d) == %d\n", i, 2, (i < 2));
> >> }
> >>
> >> return 0;
> >> }
> >>
> >> int main()
> >> {
> >> isci_pci_probe();
> >> }
> >>
> >> $ gcc bug.c
> >> $./a.out
> >> 0 < 2) == 1
> >> (1 < 2) == 1
> >> $ gcc bug.c -O2
> >> $ ./a.out
> >> (0 < 2) == 1
> >> (1 < 2) == 1
> >> Segmentation fault (core dumped)
> >
> > Your testcase is invalid:
> >
> > markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
> > markus@x4 tmp % ./a.out
> > (0 < 2) == 1
> > (1 < 2) == 1
> > bug.c:16:20: runtime error: index 2 out of bounds for type 'struct
> > isci_host *[2]'
> >
> > As Jakub Jelinek said on IRC, changing the loop to e.g.:
> >
> > for (i = 0;
> >i < 2 && (isci_host = v.hosts[i]);
> >i++) {
> >
> > fixes the issue.
>
> testcase was reduced from drivers/scsi/isci/host.h written back in
> 2011 (commit b329aff107)
> #define for_each_isci_host(id, ihost, pdev) \
> for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
> id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
> ihost = to_pci_info(pdev)->hosts[++id])
>
> yes, it does access 3rd element of 2 element array and will read junk.
>
> C standard says the behavior is undefined and comes handy in compiler defense,
> but in this case compiler has obviously all the information to make
> right decision
> instead of misoptimizing the code.
> So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
> --
Thank you for explanation!
Alexei,
Will you file a gcc bug for this case?
Thanks,
Lukasz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Why is (2 < 2) true? Is it a gcc bug?
On Friday, January 17, 2014 5:40 PM Sebastian Riemer
wrote:
> On 17.01.2014 14:55, Dorau, Lukasz wrote:
> >
> > Some additional information:
> >
> > The loop 'for' in macro ' for_each_isci_host ' defined as
> (drivers/scsi/isci/host.h:313):
> >
> > #define for_each_isci_host(id, ihost, pdev) \
> > for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
> > id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
> > ihost = to_pci_info(pdev)->hosts[++id])
> >
> > should be executed only for i = 0 and 1, because:
> > ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2
> >
> > but it was executed also for i=2 regardless the above loop's end condition.
>
> to_pci_info() can return NULL in dev_get_drvdata(). The use of
> ARRAY_SIZE() is inappropriate.
>
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
> __must_be_array(arr))
>
> #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>
> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
>
> I would say that this was supposed to trigger a build error but it
> didn't and added 1 to the loop end condition.
>
> Can you test putting SCI_MAX_CONTROLLERS to the loop end condition, please?
>
Replacing 'ARRAY_SIZE(to_pci_info(pdev)->hosts)' with 'SCI_MAX_CONTROLLERS' in
the definition of the ' for_each_isci_host ' macro does not help. I have
checked it.
The following patch helps:
http://marc.info/?l=linux-scsi&m=138987823011697&w=2
Lukasz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Why is (2 < 2) true? Is it a gcc bug?
On Friday, January 17, 2014 2:58 PM Richard Weinberger
wrote:
>
> Can you reproduce this using a standalone test?
> I.e:
> #include
>
> int main()
> {
> assert(2 < 2 != 1);
>
> return 0;
> }
>
No, I can't of course.
Lukasz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Why is (2 < 2) true? Is it a gcc bug?
On Friday, January 17, 2014 2:37 PM Dorau, Lukasz
wrote:
>
> Hi
>
> My story is very simply...
> I applied the following patch:
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
> struct
> pci_device_id *id)
> if (err)
> goto err_host_alloc;
>
> - for_each_isci_host(i, isci_host, pdev)
> + for_each_isci_host(i, isci_host, pdev) {
> + pr_err("(%d < %d) == %d\n",\
> +i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> scsi_scan_host(to_shost(isci_host));
> + }
>
> return 0;
>
> --
> 1.8.3.1
>
> Then I issued the command 'modprobe isci' on platform with two SCU controllers
> (Patsburg D or T chipset)
> and received the following, very strange, output:
>
> (0 < 2) == 1
> (1 < 2) == 1
> (2 < 2) == 1
>
> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>
> (The kernel was compiled using gcc version 4.8.2.)
>
Some additional information:
The loop 'for' in macro ' for_each_isci_host ' defined as
(drivers/scsi/isci/host.h:313):
#define for_each_isci_host(id, ihost, pdev) \
for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
ihost = to_pci_info(pdev)->hosts[++id])
should be executed only for i = 0 and 1, because:
ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2
but it was executed also for i=2 regardless the above loop's end condition.
Lukasz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Why is (2 < 2) true? Is it a gcc bug?
Hi
My story is very simply...
I applied the following patch:
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
if (err)
goto err_host_alloc;
- for_each_isci_host(i, isci_host, pdev)
+ for_each_isci_host(i, isci_host, pdev) {
+ pr_err("(%d < %d) == %d\n",\
+ i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
scsi_scan_host(to_shost(isci_host));
+ }
return 0;
--
1.8.3.1
Then I issued the command 'modprobe isci' on platform with two SCU controllers
(Patsburg D or T chipset)
and received the following, very strange, output:
(0 < 2) == 1
(1 < 2) == 1
(2 < 2) == 1
Can anyone explain why (2 < 2) is true? Is it a gcc bug?
(The kernel was compiled using gcc version 4.8.2.)
Lukasz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] isci: fix reset timeout handling
On Monday, December 09, 2013 8:47 PM Williams, Dan J wrote: > Remove an erroneous BUG_ON() in the case of a hard reset timeout. If a > SATA device is unable to restore the link in time we expect the port to > go back to the "awaiting link-up" state. Since the timeout path > notified libsas that the port is down, we want to notify libsas when it > returns and expect that the port may not be in the "resetting" state. > > Also, once we have notified that the port is down, in-progress resets should > abort immediately. Return -ENODEV from ->lldd_I_T_nexus_reset() to indicate > that libata error handling should end. > > Cc: > Reported-by: Xun Ni > Tested-by: Xun Ni > Signed-off-by: Dan Williams Acked-by: Lukasz Dorau
RE: [PATCH] scsi/isci/phy.c: Code tidiness, delete the redundant function call "sci_change_state(&iphy->sm, SCI_PHY_STOPPED)"
On 07/19/2013 Xinghai Yu wrote: > The "sci_phy_link_layer_initialization()" was called only once in > "sci_phy_initialize()" and it is called before a call of > "sci_change_state(&iphy- > >sm, SCI_PHY_STOPPED)". So the same call in the end of > "sci_phy_link_layer_initialization()" is redundant. > > Signed-off-by: Xinghai Yu > --- > drivers/scsi/isci/phy.c |3 --- > 1 files changed, 0 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/isci/phy.c b/drivers/scsi/isci/phy.c > index cb87b2e..82e8491 100644 > --- a/drivers/scsi/isci/phy.c > +++ b/drivers/scsi/isci/phy.c > @@ -309,9 +309,6 @@ sci_phy_link_layer_initialization(struct isci_phy *iphy, >*/ > writel(0, &llr->link_layer_hang_detection_timeout); > > - /* We can exit the initial state to the stopped state */ > - sci_change_state(&iphy->sm, SCI_PHY_STOPPED); > - > return SCI_SUCCESS; > } > Acked-by: Lukasz Dorau -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 14/19] scsi: Change variable type to bool
On Sunday, September 22, 2013 12:28 AM Peter Senna Tschudin
wrote:
>
> The variable success is only assigned the values true and false.
> Change its type to bool.
>
> The simplified semantic patch that find this problem is as
> follows (http://coccinelle.lip6.fr/):
>
> @exists@
> type T;
> identifier b;
> @@
> - T
> + bool
> b = ...;
> ... when any
> b = \(true\|false\)
>
> Signed-off-by: Peter Senna Tschudin
> ---
> drivers/scsi/isci/port.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
> index 13098b0..26f8e5c 100644
> --- a/drivers/scsi/isci/port.c
> +++ b/drivers/scsi/isci/port.c
> @@ -174,7 +174,7 @@ static void isci_port_link_up(struct isci_host *isci_host,
> {
> unsigned long flags;
> struct sci_port_properties properties;
> - unsigned long success = true;
> + bool success = true;
>
> dev_dbg(&isci_host->pdev->dev,
> "%s: isci_port = %p\n",
> --
> 1.8.3.1
Acked-by: Lukasz Dorau
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] scsi/isci/port_config: Fix a infinite loop.
On Wednesday, July 17, 2013 4:54 AM Xinghai Yu wrote: > > It seems the "phy_index++;" have been placed in wrong place, without it > the while circle up will do a infinite loop. > > Signed-off-by: Xinghai Yu Acked-by: Lukasz Dorau > --- > drivers/scsi/isci/port_config.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/scsi/isci/port_config.c b/drivers/scsi/isci/port_config.c > index cd962da..85c77f6 100644 > --- a/drivers/scsi/isci/port_config.c > +++ b/drivers/scsi/isci/port_config.c > @@ -311,9 +311,9 @@ sci_mpc_agent_validate_phy_configuration(struct > isci_host *ihost, > &ihost->phys[phy_index]); > > assigned_phy_mask |= (1 << phy_index); > + phy_index++; > } > > - phy_index++; > } > > return sci_port_configuration_agent_validate_ports(ihost, port_agent); > -- > 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] isci: add CONFIG_PM_SLEEP to suspend/resume functions
On Tue, 26 Mar 2013 00:01:38 -0700 Jingoo Han wrote:
> Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
> build warning when CONFIG_PM_SLEEP is not selected. This is because
> sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
> the CONFIG_PM_SLEEP is enabled.
>
> drivers/scsi/isci/init.c:725:12: warning: 'isci_suspend' defined but not used
> [-
> Wunused-function]
> drivers/scsi/isci/init.c:743:12: warning: 'isci_resume' defined but not used
> [-
> Wunused-function]
>
> Signed-off-by: Jingoo Han
Acked-by: Lukasz Dorau
> ---
> drivers/scsi/isci/init.c |6 ++
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> index 2839baa..d25d0d8 100644
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -721,7 +721,7 @@ static void isci_pci_remove(struct pci_dev *pdev)
> }
> }
>
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> static int isci_suspend(struct device *dev)
> {
> struct pci_dev *pdev = to_pci_dev(dev);
> @@ -770,18 +770,16 @@ static int isci_resume(struct device *dev)
>
> return 0;
> }
> +#endif
>
> static SIMPLE_DEV_PM_OPS(isci_pm_ops, isci_suspend, isci_resume);
> -#endif
>
> static struct pci_driver isci_pci_driver = {
> .name = DRV_NAME,
> .id_table = isci_id_table,
> .probe = isci_pci_probe,
> .remove = isci_pci_remove,
> -#ifdef CONFIG_PM
> .driver.pm = &isci_pm_ops,
> -#endif
> };
>
> static __init int isci_init(void)
> --
> 1.7.2.5
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
4 isci patches from isci-for-3.6 tag?
Hi James What are you going to do with the following four ISCI patches from isci-for-3.6 tag?: 6734092e66011def7875bd67beef889d0fee1cc9isci: add a couple __iomem annotations 67787c330762eb884bf8c169fe942263d55ec162isci: make function declaration match implementation a90037560588e51b3e98b49537799137cbfda17d isci: fix COMSAS negation timout workaround for WD SAS drives 6d70a74ffd616073a68ae0974d98819bfa8e6da6 isci: fix isci_pci_probe() generates warning on efi failure path (by: Dan Carpenter, Dave Maurer and Dan Williams) We hope they will get to the upstream kernel soon... Regards, Lukasz -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
