Re: [PATCH] staging: gdm72xx: enclose complex define statement

[Greg KH]

On Mon, Apr 20, 2015 at 10:11:51PM -0500, Jaime Arrocha wrote:
> This patch fixes the warning found by checkpatch.pl:
> ERROR: Macros with complex values should be enclosed in parentheses
> 
> Signed-off-by: Jaime Arrocha 
> ---
>  drivers/staging/gdm72xx/usb_ids.h |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/gdm72xx/usb_ids.h 
> b/drivers/staging/gdm72xx/usb_ids.h
> index 8ce544d..2b50ac6 100644
> --- a/drivers/staging/gdm72xx/usb_ids.h
> +++ b/drivers/staging/gdm72xx/usb_ids.h
> @@ -32,8 +32,8 @@
>  #define BL_PID_MASK  0xffc0
>  
>  #define USB_DEVICE_BOOTLOADER(vid, pid)  \
> - {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},\
> - {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)}
> + ({USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},   \
> + {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)})

checkpatch isn't always correct.  This is one such example.

Does this even compile?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: comedi: fix coding style errors in daqboard2000.c

[Greg KH]

On Mon, Apr 20, 2015 at 07:57:31PM -0700, Gbenga Adalumo wrote:
> The patch fixes a trailing whitespace and code indenting coding style
> errors as reported by checkpatch.pl tool.
>  Details of the lines where the fixed errors were reported are as follows:
> 
> drivers/staging/comedi/drivers/daqboard2000.c:43: ERROR: trailing whitespace
> drivers/staging/comedi/drivers/daqboard2000.c:46: ERROR: code indent
> should use tabs where possible



You don't have to list all of the individual errors, and not in a
line-wrapped way either.

It looks like you are fixing different types of errors, so please
provide different patches to do them.  Each patch should just do one
thing.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths

[Vitaly Kuznetsov]

In case there was an error reported in the response to the 
CHANNELMSG_OPENCHANNEL
call we need to do the cleanup as a vmbus_open() user won't be doing it after
receiving an error. The cleanup should be done on all failure paths.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 54da66d..836386f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -186,11 +186,12 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 
send_ringbuffer_size,
list_del(&open_info->msglistentry);
spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
-   if (err == 0)
-   newchannel->state = CHANNEL_OPENED_STATE;
+   if (err != 0)
+   goto error_gpadl;
 
+   newchannel->state = CHANNEL_OPENED_STATE;
kfree(open_info);
-   return err;
+   return 0;
 
 error1:
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] Drivers: hv: vmbus: kill tasklets on module unload

[Vitaly Kuznetsov]

Explicitly kill tasklets we create on module unload.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/vmbus_drv.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 2b56260..cf20400 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1108,6 +1108,7 @@ static void __exit vmbus_exit(void)
hv_synic_clockevents_cleanup();
vmbus_disconnect();
hv_remove_vmbus_irq();
+   tasklet_kill(&msg_dpc);
vmbus_free_channels();
if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
atomic_notifier_chain_unregister(&panic_notifier_list,
@@ -1115,8 +1116,10 @@ static void __exit vmbus_exit(void)
}
bus_unregister(&hv_bus);
hv_cleanup();
-   for_each_online_cpu(cpu)
+   for_each_online_cpu(cpu) {
+   tasklet_kill(hv_context.event_dpc[cpu]);
smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
+   }
acpi_bus_unregister_driver(&vmbus_acpi_driver);
hv_cpu_hotplug_quirk(false);
 }
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] Drivers: hv: vmbus: setup irq after synic is initialized

[Vitaly Kuznetsov]

vmbus_isr() uses synic pages which are being setup in hv_synic_alloc(), we
need to register this irq handler only after we allocate all the required
pages.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/vmbus_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index cf20400..e922804 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -822,11 +822,12 @@ static int vmbus_bus_init(int irq)
if (ret)
goto err_cleanup;
 
-   hv_setup_vmbus_irq(vmbus_isr);
-
ret = hv_synic_alloc();
if (ret)
goto err_alloc;
+
+   hv_setup_vmbus_irq(vmbus_isr);
+
/*
 * Initialize the per-cpu interrupt state and
 * connect to the host.
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths

[Dexuan Cui]

> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Tuesday, April 21, 2015 16:18
> To: KY Srinivasan
> Cc: Haiyang Zhang; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org; Dexuan Cui
> Subject: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open()
> failure paths
> 
> In case there was an error reported in the response to the
> CHANNELMSG_OPENCHANNEL
> call we need to do the cleanup as a vmbus_open() user won't be doing it
> after
> receiving an error. The cleanup should be done on all failure paths.
> 
> Signed-off-by: Vitaly Kuznetsov 
> ---
>  drivers/hv/channel.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 54da66d..836386f 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -186,11 +186,12 @@ int vmbus_open(struct vmbus_channel
> *newchannel, u32 send_ringbuffer_size,
>   list_del(&open_info->msglistentry);
>   spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock,
> flags);
> 
> - if (err == 0)
> - newchannel->state = CHANNEL_OPENED_STATE;
> + if (err != 0)
> + goto error_gpadl;
> 
> + newchannel->state = CHANNEL_OPENED_STATE;
>   kfree(open_info);
> - return err;
> + return 0;
> 
>  error1:
>   spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
> --

Thanks for the patch, Vitaly!

The patch looks good to me except for 1 small issue:
I suppose open_info->response.open_result.status is different from
Linux-style -EXXX error codes.

Maybe here we can return -EAGAIN if err != 0 ?

-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] staging: comedi: serial2002: fix Coverity "Explicit null dereference"

[Dan Carpenter]

Yeah.  That's tricky code for a static checker.  You have to know that
if the caller passes kind = 1 or 2 then range is NULL.

The caller users S2002_CFG_KIND_DIGITAL_IN and S2002_CFG_KIND_DIGITAL_OUT
for 1 and 2.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/3] Drivers: hv: vmbus: additional fixes for the setup/teardown path

[Vitaly Kuznetsov]

K. Y.,

here are 3 additional patches for your "[PATCH 0/5] Drivers: hv: vmbus: Cleanup
the vmbus unload path" series (with the fix I suggested). I tested the whole
set on Gen2 Win2012R2 guest, load/unload path seems being stable. Can you
please take a look?

Thanks,

Vitaly Kuznetsov (3):
  Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths
  Drivers: hv: vmbus: kill tasklets on module unload
  Drivers: hv: vmbus: setup irq after synic is initialized

 drivers/hv/channel.c   |  7 ---
 drivers/hv/vmbus_drv.c | 10 +++---
 2 files changed, 11 insertions(+), 6 deletions(-)

-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths

[Dan Carpenter]

On Tue, Apr 21, 2015 at 10:17:52AM +0200, Vitaly Kuznetsov wrote:
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 54da66d..836386f 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -186,11 +186,12 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 
> send_ringbuffer_size,
>   list_del(&open_info->msglistentry);
>   spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
>  
> - if (err == 0)
> - newchannel->state = CHANNEL_OPENED_STATE;
> + if (err != 0)

Doesn't the double negative not make it slightly more confusing?

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: comedi: fix coding style errors in daqboard2000.c

[Dan Carpenter]

Oh.  Ok.  Resend with a corrected changelog then.

But don't include all the spammy output just one or two lines or a
summary.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: comedi: fix coding style errors in daqboard2000.c

[Ian Abbott]

On 21/04/15 08:50, Greg KH wrote:

On Mon, Apr 20, 2015 at 07:57:31PM -0700, Gbenga Adalumo wrote:

The patch fixes a trailing whitespace and code indenting coding style
errors as reported by checkpatch.pl tool.
  Details of the lines where the fixed errors were reported are as follows:

drivers/staging/comedi/drivers/daqboard2000.c:43: ERROR: trailing whitespace
drivers/staging/comedi/drivers/daqboard2000.c:46: ERROR: code indent
should use tabs where possible




You don't have to list all of the individual errors, and not in a
line-wrapped way either.

It looks like you are fixing different types of errors, so please
provide different patches to do them.  Each patch should just do one
thing.


All these "errors" occur in a single block comment.  It would be 
preferable if that block comment were reformatted to use the usual 
block-comment style:


/*
 * blah
 * blah
 */

This has already been done for some comedi drivers.

Thanks,
Ian A.

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/3] staging: comedi: Coverity fixes

[Ian Abbott]

On 20/04/15 19:49, H Hartley Sweeten wrote:

Fix a couple issues reported by Coverity.

H Hartley Sweeten (3):
   staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()
   staging: comedi: ni_nio_common: don't write non-existing caldac's
   staging: comedi: serial2002: fix Coverity "Explicit null dereference"

  drivers/staging/comedi/drivers/comedi_bond.c   | 3 ++-
  drivers/staging/comedi/drivers/ni_mio_common.c | 4 
  drivers/staging/comedi/drivers/serial2002.c| 2 +-
  3 files changed, 7 insertions(+), 2 deletions(-)


Reviewed-by: Ian Abbott 

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()

[Dan Carpenter]

On Mon, Apr 20, 2015 at 11:49:04AM -0700, H Hartley Sweeten wrote:
> 'b_chans' may be a valud up to 32. 'b_mask' is an unsigned int and a left 
> shift of
> more than 31 bits has undefined behavior. Fix the calc so it works correctly 
> with
> a 'b_chans' of 32..
> 
> Reported-by: coverity (CID 1192244)
> Signed-off-by: H Hartley Sweeten 
> Cc: Ian Abbott 
> Cc: Greg Kroah-Hartman 
> ---
>  drivers/staging/comedi/drivers/comedi_bond.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/comedi/drivers/comedi_bond.c 
> b/drivers/staging/comedi/drivers/comedi_bond.c
> index 96db0c2..50b76ec 100644
> --- a/drivers/staging/comedi/drivers/comedi_bond.c
> +++ b/drivers/staging/comedi/drivers/comedi_bond.c
> @@ -101,7 +101,8 @@ static int bonding_dio_insn_bits(struct comedi_device 
> *dev,
>   b_chans = bdev->nchans - base_chan;
>   if (b_chans > n_left)
>   b_chans = n_left;
> - b_mask = (1U << b_chans) - 1;
> + b_mask = (b_chans < 32) ? ((1 << b_chans) - 1)
> + : 0x;

Is that really an improvement?  The original code was actually defined.

1U << 32 is actually defined.  It is zero.  Which works for us.
1 << 32 is undefined.  It could be anything.
1 << 31 is INT_MIN.

I think INT_MIN - 1 might be undefined.  But I'm not sure.  Although, in
my testing "INT_MIN - 1" and "(uint)INT_MIN - 1" are the same which is
what we intended so maybe the new code is fine.

regards,
dan carpenter

>   b_write_mask = (write_mask >> n_done) & b_mask;
>   b_data_bits = (data_bits >> n_done) & b_mask;
>   /* Read/Write the new digital lines. */
> -- 
> 2.3.0
> 
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths

[Vitaly Kuznetsov]

Dexuan Cui  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
>> Sent: Tuesday, April 21, 2015 16:18
>> To: KY Srinivasan
>> Cc: Haiyang Zhang; de...@linuxdriverproject.org; linux-
>> ker...@vger.kernel.org; Dexuan Cui
>> Subject: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open()
>> failure paths
>> 
>> In case there was an error reported in the response to the
>> CHANNELMSG_OPENCHANNEL
>> call we need to do the cleanup as a vmbus_open() user won't be doing it
>> after
>> receiving an error. The cleanup should be done on all failure paths.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>> ---
>>  drivers/hv/channel.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
>> index 54da66d..836386f 100644
>> --- a/drivers/hv/channel.c
>> +++ b/drivers/hv/channel.c
>> @@ -186,11 +186,12 @@ int vmbus_open(struct vmbus_channel
>> *newchannel, u32 send_ringbuffer_size,
>>  list_del(&open_info->msglistentry);
>>  spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock,
>> flags);
>> 
>> -if (err == 0)
>> -newchannel->state = CHANNEL_OPENED_STATE;
>> +if (err != 0)
>> +goto error_gpadl;
>> 
>> +newchannel->state = CHANNEL_OPENED_STATE;
>>  kfree(open_info);
>> -return err;
>> +return 0;
>> 
>>  error1:
>>  spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
>> --
>
> Thanks for the patch, Vitaly!
>
> The patch looks good to me except for 1 small issue:
> I suppose open_info->response.open_result.status is different from
> Linux-style -EXXX error codes.
>
> Maybe here we can return -EAGAIN if err != 0 ?

I think I inspected all vmbus_open() callers and they all just check ret
!= 0 but I agree we'd better return some -E code here. I'm not
exactly sure if open_result.status != 0 usually means a permanent or a
temporary failure but if it's temporary -EAGAIN here seems reasonable.

I'll send v2, thanks for the review!

>
> -- Dexuan

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths

[Vitaly Kuznetsov]

Dan Carpenter  writes:

> On Tue, Apr 21, 2015 at 10:17:52AM +0200, Vitaly Kuznetsov wrote:
>> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
>> index 54da66d..836386f 100644
>> --- a/drivers/hv/channel.c
>> +++ b/drivers/hv/channel.c
>> @@ -186,11 +186,12 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 
>> send_ringbuffer_size,
>>  list_del(&open_info->msglistentry);
>>  spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
>>  
>> -if (err == 0)
>> -newchannel->state = CHANNEL_OPENED_STATE;
>> +if (err != 0)
>
> Doesn't the double negative not make it slightly more confusing?
>

Point taken, will fix in v2 :-)

> regards,
> dan carpenter

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/2] staging: comedi: hide subdevice runflags stuff

[Ian Abbott]

Keep the details of the comedi subdevice `runflags` member local to
"comedi_fops.c".  In particular, the usage of the
`COMEDI_SRF_FREE_SPRIV` run-flag doesn't really fit in all that well
with the others.  It's used as a marker to indicate the subdevice's
`private` pointer can be automatically freed by the subdevice
clean-up code, whereas the others are associated with the operation of
asynchronous comedi commands.  Abstract it's usage away in a couple of
new wrapper functions.

1) staging: comedi: wrap COMEDI_SRF_FREE_SPRIV usage
2) staging: comedi: move COMEDI_SRF_... macros to "comedi_fops.c"

 drivers/staging/comedi/comedi_fops.c   | 41 --
 drivers/staging/comedi/comedi_internal.h   |  1 +
 drivers/staging/comedi/comedidev.h | 18 +-
 drivers/staging/comedi/drivers.c   |  2 +-
 .../staging/comedi/drivers/amplc_dio200_common.c   |  6 ++--
 5 files changed, 45 insertions(+), 23 deletions(-)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] staging: comedi: move COMEDI_SRF_... macros to "comedi_fops.c"

[Ian Abbott]

The `COMEDI_SRF_...` macros define flag combinations in the `runflags`
member of `struct comedi_subdevice`.  They are only used directly in
"comedi_fops.c", so move them to there.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 17 +
 drivers/staging/comedi/comedidev.h   | 17 -
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index fc339c5..6f269cc 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -44,6 +44,23 @@
 #include "comedi_internal.h"
 
 /**
+ * comedi_subdevice "runflags"
+ * @COMEDI_SRF_RT: DEPRECATED: command is running real-time
+ * @COMEDI_SRF_ERROR:  indicates an COMEDI_CB_ERROR event has occurred
+ * since the last command was started
+ * @COMEDI_SRF_RUNNING:command is running
+ * @COMEDI_SRF_FREE_SPRIV: free s->private on detach
+ *
+ * @COMEDI_SRF_BUSY_MASK:  runflags that indicate the subdevice is "busy"
+ */
+#define COMEDI_SRF_RT  BIT(1)
+#define COMEDI_SRF_ERROR   BIT(2)
+#define COMEDI_SRF_RUNNING BIT(27)
+#define COMEDI_SRF_FREE_SPRIV  BIT(31)
+
+#define COMEDI_SRF_BUSY_MASK   (COMEDI_SRF_ERROR | COMEDI_SRF_RUNNING)
+
+/**
  * struct comedi_file - per-file private data for comedi device
  * @dev: comedi_device struct
  * @read_subdev: current "read" subdevice
diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index 52071f7..28f2606 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -303,23 +303,6 @@ void comedi_event(struct comedi_device *dev, struct 
comedi_subdevice *s);
 struct comedi_device *comedi_dev_get_from_minor(unsigned minor);
 int comedi_dev_put(struct comedi_device *dev);
 
-/**
- * comedi_subdevice "runflags"
- * @COMEDI_SRF_RT: DEPRECATED: command is running real-time
- * @COMEDI_SRF_ERROR:  indicates an COMEDI_CB_ERROR event has occurred
- * since the last command was started
- * @COMEDI_SRF_RUNNING:command is running
- * @COMEDI_SRF_FREE_SPRIV: free s->private on detach
- *
- * @COMEDI_SRF_BUSY_MASK:  runflags that indicate the subdevice is "busy"
- */
-#define COMEDI_SRF_RT  BIT(1)
-#define COMEDI_SRF_ERROR   BIT(2)
-#define COMEDI_SRF_RUNNING BIT(27)
-#define COMEDI_SRF_FREE_SPRIV  BIT(31)
-
-#define COMEDI_SRF_BUSY_MASK   (COMEDI_SRF_ERROR | COMEDI_SRF_RUNNING)
-
 bool comedi_is_subdevice_running(struct comedi_subdevice *s);
 
 void *comedi_alloc_spriv(struct comedi_subdevice *s, size_t size);
-- 
2.1.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: comedi: wrap COMEDI_SRF_FREE_SPRIV usage

[Ian Abbott]

The `COMEDI_SRF_FREE_SPRIV` flag in the `runflags` member of `struct
comedi_subdevice` indicates that the memory pointed to by the `private`
member can be automatically freed by the comedi core on subdevice
clean-up (when the low-level comedi device is being "detached").  the
flag doesn't really belong in `runflags`, but it was somewhere
convenient to keep it without having to add a new member to the
structure.

Rather than access the `COMEDI_SRF_FREE_SPRIV` flag directly, use some
new wrapper functions:

* comedi_can_auto_free_spriv(s) - checks whether the subdevice's
  `s->private` points to memory that can be freed automatically.
* comedi_set_spriv_auto_free(s) - marks the subdevice as having a
  `s->private` that points to memory that can be freed automatically.

Export `comedi_set_spriv_auto_free()` for use by the low-level comedi
driver modules, in particular the "amplc_dio200_common" module.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c   | 24 --
 drivers/staging/comedi/comedi_internal.h   |  1 +
 drivers/staging/comedi/comedidev.h |  1 +
 drivers/staging/comedi/drivers.c   |  2 +-
 .../staging/comedi/drivers/amplc_dio200_common.c   |  6 +++---
 5 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index e78ddbe..fc339c5 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -679,8 +679,28 @@ static bool comedi_is_subdevice_idle(struct 
comedi_subdevice *s)
return !(runflags & COMEDI_SRF_BUSY_MASK);
 }
 
+bool comedi_can_auto_free_spriv(struct comedi_subdevice *s)
+{
+   unsigned runflags = __comedi_get_subdevice_runflags(s);
+
+   return runflags & COMEDI_SRF_FREE_SPRIV;
+}
+
+/**
+ * comedi_set_spriv_auto_free - mark subdevice private data as freeable
+ * @s: comedi_subdevice struct
+ *
+ * Mark the subdevice as having a pointer to private data that can be
+ * automatically freed by the comedi core during the detach.
+ */
+void comedi_set_spriv_auto_free(struct comedi_subdevice *s)
+{
+   __comedi_set_subdevice_runflags(s, COMEDI_SRF_FREE_SPRIV);
+}
+EXPORT_SYMBOL_GPL(comedi_set_spriv_auto_free);
+
 /**
- * comedi_alloc_spriv() - Allocate memory for the subdevice private data.
+ * comedi_alloc_spriv - Allocate memory for the subdevice private data.
  * @s: comedi_subdevice struct
  * @size: size of the memory to allocate
  *
@@ -691,7 +711,7 @@ void *comedi_alloc_spriv(struct comedi_subdevice *s, size_t 
size)
 {
s->private = kzalloc(size, GFP_KERNEL);
if (s->private)
-   s->runflags |= COMEDI_SRF_FREE_SPRIV;
+   comedi_set_spriv_auto_free(s);
return s->private;
 }
 EXPORT_SYMBOL_GPL(comedi_alloc_spriv);
diff --git a/drivers/staging/comedi/comedi_internal.h 
b/drivers/staging/comedi/comedi_internal.h
index 3b91853..cd9437f 100644
--- a/drivers/staging/comedi/comedi_internal.h
+++ b/drivers/staging/comedi/comedi_internal.h
@@ -33,6 +33,7 @@ struct comedi_buf_map *comedi_buf_map_from_subdev_get(
struct comedi_subdevice *s);
 unsigned int comedi_buf_write_n_allocated(struct comedi_subdevice *s);
 void comedi_device_cancel_all(struct comedi_device *dev);
+bool comedi_can_auto_free_spriv(struct comedi_subdevice *s);
 
 extern unsigned int comedi_default_buf_size_kb;
 extern unsigned int comedi_default_buf_maxsize_kb;
diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index dfab5a8..52071f7 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -323,6 +323,7 @@ int comedi_dev_put(struct comedi_device *dev);
 bool comedi_is_subdevice_running(struct comedi_subdevice *s);
 
 void *comedi_alloc_spriv(struct comedi_subdevice *s, size_t size);
+void comedi_set_spriv_auto_free(struct comedi_subdevice *s);
 
 int comedi_check_chanlist(struct comedi_subdevice *s,
  int n,
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 57dcffe..ed0b60c 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -125,7 +125,7 @@ static void comedi_device_detach_cleanup(struct 
comedi_device *dev)
if (dev->subdevices) {
for (i = 0; i < dev->n_subdevices; i++) {
s = &dev->subdevices[i];
-   if (s->runflags & COMEDI_SRF_FREE_SPRIV)
+   if (comedi_can_auto_free_spriv(s))
kfree(s->private);
comedi_free_subdevice_minor(s);
if (s->async) {
diff --git a/drivers/staging/comedi/drivers/amplc_dio200_common.c 
b/drivers/staging/comedi/drivers/amplc_dio200_common.c
index d15a3dc..3a8b3f2 100644
--- a/drivers/staging/comedi/drivers/amplc_dio200_common.c
+++ b/drivers/staging/comedi/drivers/amplc_dio200_common.c
@@ -5

Re: [PATCH 1/3] staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()

[Ian Abbott]

On 21/04/15 12:13, Dan Carpenter wrote:

On Mon, Apr 20, 2015 at 11:49:04AM -0700, H Hartley Sweeten wrote:

'b_chans' may be a valud up to 32. 'b_mask' is an unsigned int and a left shift 
of
more than 31 bits has undefined behavior. Fix the calc so it works correctly 
with
a 'b_chans' of 32..

Reported-by: coverity (CID 1192244)
Signed-off-by: H Hartley Sweeten 
Cc: Ian Abbott 
Cc: Greg Kroah-Hartman 
---
  drivers/staging/comedi/drivers/comedi_bond.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/comedi_bond.c 
b/drivers/staging/comedi/drivers/comedi_bond.c
index 96db0c2..50b76ec 100644
--- a/drivers/staging/comedi/drivers/comedi_bond.c
+++ b/drivers/staging/comedi/drivers/comedi_bond.c
@@ -101,7 +101,8 @@ static int bonding_dio_insn_bits(struct comedi_device *dev,
b_chans = bdev->nchans - base_chan;
if (b_chans > n_left)
b_chans = n_left;
-   b_mask = (1U << b_chans) - 1;
+   b_mask = (b_chans < 32) ? ((1 << b_chans) - 1)
+   : 0x;


Is that really an improvement?  The original code was actually defined.

1U << 32 is actually defined.  It is zero.  Which works for us.


According to the C standards it is undefined (for 32-bit unsigned int). 
 See the late draft of ISO/IEC 9899:2011 (dubbed C11) at 
, §6.5.7, 
Semantics:


3. The integer promotions are performed on each of the operands. The 
type of the result is that of the promoted left operand.If the value of 
the right operand is negative or is greater than or equal to the width 
of the promoted left operand, the behavior is undefined.



1 << 32 is undefined.  It could be anything.
1 << 31 is INT_MIN.


Technically, 1 << 31 is undefined as 1*2^32 is not representable as a 
non-negative value (see item 4 of the semantics), but in practice will 
be INT_MIN for a 32-bit, 2's complement, signed int.




I think INT_MIN - 1 might be undefined.  But I'm not sure.  Although, in
my testing "INT_MIN - 1" and "(uint)INT_MIN - 1" are the same which is
what we intended so maybe the new code is fine.


It would be better to use (1U << b_chans) instead of (1 << b_chans) in 
the code above, or possibly the slightly more obtuse:


b_mask = ((b_chans < 32) ? 1 << b_chans : 0) - 1;

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()

[Dan Carpenter]

On Tue, Apr 21, 2015 at 01:52:09PM +0100, Ian Abbott wrote:
> >Is that really an improvement?  The original code was actually defined.
> >
> >1U << 32 is actually defined.  It is zero.  Which works for us.
> 
> According to the C standards it is undefined (for 32-bit unsigned
> int).  See the late draft of ISO/IEC 9899:2011 (dubbed C11) at
> , §6.5.7,
> Semantics:
> 
> 3. The integer promotions are performed on each of the operands. The
> type of the result is that of the promoted left operand.If the value
> of the right operand is negative or is greater than or equal to the
> width of the promoted left operand, the behavior is undefined.
>

Yeah.  You're right.

> 
> It would be better to use (1U << b_chans) instead of (1 << b_chans)
> in the code above, or possibly the slightly more obtuse:
> 
>   b_mask = ((b_chans < 32) ? 1 << b_chans : 0) - 1;

I like just switching Hartley's 1 to 1U.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/6] Drivers: hv: vmbus: fair round robin algorithm for vmbus_get_outgoing_channel()

[Vitaly Kuznetsov]

This series is a continuation of the "Drivers: hv: vmbus: Use a round-robin
algorithm for picking the outgoing channel" work. It is supposed to bring two
significant changes:
1) Subchannels for a channel are distributed evenly across all vcpus we have.
   Currently we try to distribute all channels (including subchannels) across
   all vcpus, this approach doesn't guarantee that the particular channel's
   subchannels will be distributed in the same way as we process all offer
   requests in some random order. (Patch 05)
2) Channel picking based on the current vcpu is dropped from
   vmbus_get_outgoing_channel() in favor of a fair round robin. (Patch 06)

Patches 01 - 04 are cleanup/refactoring.

It would be also possible to make the change the other way around: always pick
the subchannel based on the current vcpu (and fallback to a round robin in case
there is no subchannel for the current vcpu). Please let me know if you think
it is preferable.

Vitaly Kuznetsov (6):
  Drivers: hv: vmbus: unify calls to percpu_channel_enq()
  Drivers: hv: vmbus: briefly comment num_sc and next_oc
  Drivers: hv: vmbus: decrease num_sc on subchannel removal
  Drivers: hv: vmbus: move init_vp_index() call to vmbus_process_offer()
  Drivers: hv: vmbus: distribute subchannels among all vcpus
  Drivers: hv: vmbus: do a fair round robin when selecting an outgoing
channel

 drivers/hv/channel_mgmt.c | 248 +++---
 include/linux/hyperv.h|  12 ++-
 2 files changed, 135 insertions(+), 125 deletions(-)

-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 5/6] Drivers: hv: vmbus: distribute subchannels among all vcpus

[Vitaly Kuznetsov]

Primary channels are distributed evenly across all vcpus we have. When the host
asks us to create subchannels it usually makes us num_cpus-1 offers and we are
supposed to distribute the work evenly among the channel itself and all its
subchannels. Make sure they are all assigned to different vcpus.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 8f2761f..daa6417 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -270,6 +270,8 @@ static void init_vp_index(struct vmbus_channel *channel,
int i;
bool perf_chn = false;
u32 max_cpus = num_online_cpus();
+   struct vmbus_channel *primary = channel->primary_channel, *prev;
+   unsigned long flags;
 
for (i = IDE; i < MAX_PERF_CHN; i++) {
if (!memcmp(type_guid->b, hp_devs[i].guid,
@@ -290,7 +292,32 @@ static void init_vp_index(struct vmbus_channel *channel,
channel->target_vp = 0;
return;
}
-   cur_cpu = (++next_vp % max_cpus);
+
+   /*
+* Primary channels are distributed evenly across all vcpus we have.
+* When the host asks us to create subchannels it usually makes us
+* num_cpus-1 offers and we are supposed to distribute the work evenly
+* among the channel itself and all its subchannels. Make sure they are
+* all assigned to different vcpus.
+*/
+   if (!primary)
+   cur_cpu = (++next_vp % max_cpus);
+   else {
+   /*
+* Let's assign the first subchannel of a channel to the
+* primary->target_cpu+1 and all the subsequent channels to
+* the prev->target_cpu+1.
+*/
+   spin_lock_irqsave(&primary->lock, flags);
+   if (primary->num_sc == 1)
+   cur_cpu = (primary->target_cpu + 1) % max_cpus;
+   else {
+   prev = list_prev_entry(channel, sc_list);
+   cur_cpu = (prev->target_cpu + 1) % max_cpus;
+   }
+   spin_unlock_irqrestore(&primary->lock, flags);
+   }
+
channel->target_cpu = cur_cpu;
channel->target_vp = hv_context.vp_index[cur_cpu];
 }
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 6/6] Drivers: hv: vmbus: do a fair round robin when selecting an outgoing channel

[Vitaly Kuznetsov]

vmbus_get_outgoing_channel() implements the following algorithm for selecting
an outgoing channel (despite the comment before the function saying it
distributes the load equally):
1) If we have no subchannels return the primary channel;
2) If primary->next_oc is grater than primary->num_sc reset the primary->next_oc
   to 0 and return the primary channel;
3) Aim for the primary->next_oc subchannel, increment primary->next_oc;
4) Loop through all opened subchannels. If we see a channel which has
 target_cpu == current_cpu return it. If we reached the primary->next_oc'th
open subchannel return it;
5) Return the primary channel.
The implementation also skips the subchannel No. 0 unless it matches the current
cpu as we assign i to 1 in the initialization.

This is not a fair round robin as subchannels in the beginning of the list are
more likely to be returned and checking for current cpu aslo creates additional
complexity. Simplify the vmbus_get_outgoing_channel() function, make it do what
the comment before it says.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 27 +--
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index daa6417..df82442 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -827,39 +827,30 @@ cleanup:
 struct vmbus_channel *vmbus_get_outgoing_channel(struct vmbus_channel *primary)
 {
struct list_head *cur, *tmp;
-   int cur_cpu;
struct vmbus_channel *cur_channel;
-   struct vmbus_channel *outgoing_channel = primary;
-   int next_channel;
-   int i = 1;
+   int i = 0;
 
if (list_empty(&primary->sc_list))
-   return outgoing_channel;
+   return primary;
 
-   next_channel = primary->next_oc++;
-
-   if (next_channel > (primary->num_sc)) {
+   if (primary->next_oc > primary->num_sc) {
primary->next_oc = 0;
-   return outgoing_channel;
+   return primary;
}
 
-   cur_cpu = hv_context.vp_index[get_cpu()];
-   put_cpu();
list_for_each_safe(cur, tmp, &primary->sc_list) {
+   i++;
cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
if (cur_channel->state != CHANNEL_OPENED_STATE)
continue;
 
-   if (cur_channel->target_vp == cur_cpu)
-   return cur_channel;
-
-   if (i == next_channel)
+   if (i > primary->next_oc) {
+   primary->next_oc = i;
return cur_channel;
-
-   i++;
+   }
}
 
-   return outgoing_channel;
+   return primary;
 }
 EXPORT_SYMBOL_GPL(vmbus_get_outgoing_channel);
 
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/6] Drivers: hv: vmbus: move init_vp_index() call to vmbus_process_offer()

[Vitaly Kuznetsov]

We need to call init_vp_index() after we added the channel to the appropriate
list (global or subchannel) to be able to use this information when assigning
the channel to the particular vcpu. To do so we need to move a couple of
functions around. The only real change is the init_vp_index() call. This is a
small refactoring without a functional change.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 142 +++---
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 8b4b561..8f2761f 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -226,6 +226,75 @@ void vmbus_free_channels(void)
}
 }
 
+enum {
+   IDE = 0,
+   SCSI,
+   NIC,
+   MAX_PERF_CHN,
+};
+
+/*
+ * This is an array of device_ids (device types) that are performance critical.
+ * We attempt to distribute the interrupt load for these devices across
+ * all available CPUs.
+ */
+static const struct hv_vmbus_device_id hp_devs[] = {
+   /* IDE */
+   { HV_IDE_GUID, },
+   /* Storage - SCSI */
+   { HV_SCSI_GUID, },
+   /* Network */
+   { HV_NIC_GUID, },
+   /* NetworkDirect Guest RDMA */
+   { HV_ND_GUID, },
+};
+
+/*
+ * We use this state to statically distribute the channel interrupt load.
+ */
+static u32  next_vp;
+
+/*
+ * Starting with Win8, we can statically distribute the incoming
+ * channel interrupt load by binding a channel to VCPU. We
+ * implement here a simple round robin scheme for distributing
+ * the interrupt load.
+ * We will bind channels that are not performance critical to cpu 0 and
+ * performance critical channels (IDE, SCSI and Network) will be uniformly
+ * distributed across all available CPUs.
+ */
+static void init_vp_index(struct vmbus_channel *channel,
+ const uuid_le *type_guid)
+{
+   u32 cur_cpu;
+   int i;
+   bool perf_chn = false;
+   u32 max_cpus = num_online_cpus();
+
+   for (i = IDE; i < MAX_PERF_CHN; i++) {
+   if (!memcmp(type_guid->b, hp_devs[i].guid,
+sizeof(uuid_le))) {
+   perf_chn = true;
+   break;
+   }
+   }
+   if ((vmbus_proto_version == VERSION_WS2008) ||
+   (vmbus_proto_version == VERSION_WIN7) || (!perf_chn)) {
+   /*
+* Prior to win8, all channel interrupts are
+* delivered on cpu 0.
+* Also if the channel is not a performance critical
+* channel, bind it to cpu 0.
+*/
+   channel->target_cpu = 0;
+   channel->target_vp = 0;
+   return;
+   }
+   cur_cpu = (++next_vp % max_cpus);
+   channel->target_cpu = cur_cpu;
+   channel->target_vp = hv_context.vp_index[cur_cpu];
+}
+
 /*
  * vmbus_process_offer - Process the offer by creating a channel/device
  * associated with this offer
@@ -272,6 +341,8 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
goto err_free_chan;
}
 
+   init_vp_index(newchannel, &newchannel->offermsg.offer.if_type);
+
if (newchannel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(newchannel->target_cpu,
@@ -338,75 +409,6 @@ err_free_chan:
free_channel(newchannel);
 }
 
-enum {
-   IDE = 0,
-   SCSI,
-   NIC,
-   MAX_PERF_CHN,
-};
-
-/*
- * This is an array of device_ids (device types) that are performance critical.
- * We attempt to distribute the interrupt load for these devices across
- * all available CPUs.
- */
-static const struct hv_vmbus_device_id hp_devs[] = {
-   /* IDE */
-   { HV_IDE_GUID, },
-   /* Storage - SCSI */
-   { HV_SCSI_GUID, },
-   /* Network */
-   { HV_NIC_GUID, },
-   /* NetworkDirect Guest RDMA */
-   { HV_ND_GUID, },
-};
-
-
-/*
- * We use this state to statically distribute the channel interrupt load.
- */
-static u32  next_vp;
-
-/*
- * Starting with Win8, we can statically distribute the incoming
- * channel interrupt load by binding a channel to VCPU. We
- * implement here a simple round robin scheme for distributing
- * the interrupt load.
- * We will bind channels that are not performance critical to cpu 0 and
- * performance critical channels (IDE, SCSI and Network) will be uniformly
- * distributed across all available CPUs.
- */
-static void init_vp_index(struct vmbus_channel *channel, const uuid_le 
*type_guid)
-{
-   u32 cur_cpu;
-   int i;
-   bool perf_chn = false;
-   u32 max_cpus = num_online_cpus();
-
-   for (i = IDE; i < MAX_PERF_CHN; i++) {
-   if (!memcmp(type_guid->b, hp_devs[i].guid,
-sizeof(uuid_le))) {
-   perf_chn = true;
-   break;
-   }
-   }

[PATCH 3/6] Drivers: hv: vmbus: decrease num_sc on subchannel removal

[Vitaly Kuznetsov]

It is unlikely that that host will ask us to close only one subchannel for a
device but let's be consistent. Do both num_sc++ and num_sc-- with
channel->lock to be on the safe side.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index b28cbdf..8b4b561 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -205,6 +205,7 @@ void hv_process_channel_removal(struct vmbus_channel 
*channel, u32 relid)
primary_channel = channel->primary_channel;
spin_lock_irqsave(&primary_channel->lock, flags);
list_del(&channel->sc_list);
+   channel->num_sc--;
spin_unlock_irqrestore(&primary_channel->lock, flags);
}
free_channel(channel);
@@ -265,8 +266,8 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
newchannel->primary_channel = channel;
spin_lock_irqsave(&channel->lock, flags);
list_add_tail(&newchannel->sc_list, &channel->sc_list);
-   spin_unlock_irqrestore(&channel->lock, flags);
channel->num_sc++;
+   spin_unlock_irqrestore(&channel->lock, flags);
} else
goto err_free_chan;
}
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/6] Drivers: hv: vmbus: unify calls to percpu_channel_enq()

[Vitaly Kuznetsov]

Remove some code duplication, no functional change intended.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel_mgmt.c | 51 +--
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 4b9d89a..b28cbdf 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -233,7 +233,6 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
 {
struct vmbus_channel *channel;
bool fnew = true;
-   bool enq = false;
unsigned long flags;
 
/* Make sure this is a new offer */
@@ -249,25 +248,12 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
}
}
 
-   if (fnew) {
+   if (fnew)
list_add_tail(&newchannel->listentry,
  &vmbus_connection.chn_list);
-   enq = true;
-   }
 
spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
 
-   if (enq) {
-   if (newchannel->target_cpu != get_cpu()) {
-   put_cpu();
-   smp_call_function_single(newchannel->target_cpu,
-percpu_channel_enq,
-newchannel, true);
-   } else {
-   percpu_channel_enq(newchannel);
-   put_cpu();
-   }
-   }
if (!fnew) {
/*
 * Check to see if this is a sub-channel.
@@ -280,26 +266,19 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
spin_lock_irqsave(&channel->lock, flags);
list_add_tail(&newchannel->sc_list, &channel->sc_list);
spin_unlock_irqrestore(&channel->lock, flags);
-
-   if (newchannel->target_cpu != get_cpu()) {
-   put_cpu();
-   smp_call_function_single(newchannel->target_cpu,
-percpu_channel_enq,
-newchannel, true);
-   } else {
-   percpu_channel_enq(newchannel);
-   put_cpu();
-   }
-
-   newchannel->state = CHANNEL_OPEN_STATE;
channel->num_sc++;
-   if (channel->sc_creation_callback != NULL)
-   channel->sc_creation_callback(newchannel);
-
-   return;
-   }
+   } else
+   goto err_free_chan;
+   }
 
-   goto err_free_chan;
+   if (newchannel->target_cpu != get_cpu()) {
+   put_cpu();
+   smp_call_function_single(newchannel->target_cpu,
+percpu_channel_enq,
+newchannel, true);
+   } else {
+   percpu_channel_enq(newchannel);
+   put_cpu();
}
 
/*
@@ -309,6 +288,12 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
 */
newchannel->state = CHANNEL_OPEN_STATE;
 
+   if (!fnew) {
+   if (channel->sc_creation_callback != NULL)
+   channel->sc_creation_callback(newchannel);
+   return;
+   }
+
/*
 * Start the process of binding this offer to the driver
 * We need to set the DeviceObject field before calling
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/6] Drivers: hv: vmbus: briefly comment num_sc and next_oc

[Vitaly Kuznetsov]

next_oc and num_sc fields of struct vmbus_channel deserve a description. Move
them closer to sc_list as these fields are related to it.

Signed-off-by: Vitaly Kuznetsov 
---
 include/linux/hyperv.h | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index c739cba..cce7f66 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -727,6 +727,15 @@ struct vmbus_channel {
 */
struct list_head sc_list;
/*
+* Current number of sub-channels.
+*/
+   int num_sc;
+   /*
+* Number of a sub-channel (position within sc_list) which is supposed
+* to be used as the next outgoing channel.
+*/
+   int next_oc;
+   /*
 * The primary channel this sub-channel belongs to.
 * This will be NULL for the primary channel.
 */
@@ -740,9 +749,6 @@ struct vmbus_channel {
 * link up channels based on their CPU affinity.
 */
struct list_head percpu_list;
-
-   int num_sc;
-   int next_oc;
 };
 
 static inline void set_channel_read_state(struct vmbus_channel *c, bool state)
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: enclose complex define statement

[Jaime Arrocha]

 On Tue, 21 Apr 2015 07:40:15 + Greg KH 
wrote  
 > On Mon, Apr 20, 2015 at 10:11:51PM -0500, Jaime Arrocha wrote: 
 > > This patch fixes the warning found by checkpatch.pl: 
 > > ERROR: Macros with complex values should be enclosed in parentheses 
 > >  
 > > Signed-off-by: Jaime Arrocha  
 > > --- 
 > >  drivers/staging/gdm72xx/usb_ids.h |4 ++-- 
 > >  1 file changed, 2 insertions(+), 2 deletions(-) 
 > >  
 > > diff --git a/drivers/staging/gdm72xx/usb_ids.h 
 > > b/drivers/staging/gdm72xx/usb_ids.h 
 > > index 8ce544d..2b50ac6 100644 
 > > --- a/drivers/staging/gdm72xx/usb_ids.h 
 > > +++ b/drivers/staging/gdm72xx/usb_ids.h 
 > > @@ -32,8 +32,8 @@ 
 > >  #define BL_PID_MASK0xffc0 
 > >   
 > >  #define USB_DEVICE_BOOTLOADER(vid, pid)\ 
 > > -{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},\ 
 > > -{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)} 
 > > +({USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},\ 
 > > +{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)}) 
 >  
 > checkpatch isn't always correct.  This is one such example. 
 >  
 > Does this even compile? 
 > 

Yes. It did. I compiled the module against 3.2.0-4-amd64 from Debian and 4.0.0 
vanilla from kernel.org. One thing that I don't understand is this:

[jaime@hpsylinux staging]$ make -C /lib/modules/3.2.0-4-amd64/build 
M=$PWD/drivers/staging/gdm72xx/ modules
make: Entering directory `/usr/src/linux-headers-3.2.0-4-amd64'
  Building modules, stage 2.
  MODPOST 0 modules
make: Leaving directory `/usr/src/linux-headers-3.2.0-4-amd64'
[jaime@hpsylinux staging]$ make -C /lib/modules/4.0.0/build 
M=$PWD/drivers/staging/gdm72xx/ modules
make: Entering directory `/home/jaime/Pprojects/linux_kernel/linux-4.0'
  Building modules, stage 2.
  MODPOST 0 modules
make: Leaving directory `/home/jaime/Pprojects/linux_kernel/linux-4.0'

MODPOST 0 modules? I get the same result without making the patch changes. To 
resolve this I'll test this on another machine.

I compiled the whole 4.0.0 kernel and got the image with no problems after 
making the patch changes.

Thank you for your time.

-Jaime

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: gdm72xx: enclose complex define statement

[Greg KH]

On Tue, Apr 21, 2015 at 02:21:32PM +, Jaime Arrocha wrote:
> 
> 
>  On Tue, 21 Apr 2015 07:40:15 + Greg KH 
> wrote  
>  > On Mon, Apr 20, 2015 at 10:11:51PM -0500, Jaime Arrocha wrote: 
>  > > This patch fixes the warning found by checkpatch.pl: 
>  > > ERROR: Macros with complex values should be enclosed in parentheses 
>  > >  
>  > > Signed-off-by: Jaime Arrocha  
>  > > --- 
>  > >  drivers/staging/gdm72xx/usb_ids.h |4 ++-- 
>  > >  1 file changed, 2 insertions(+), 2 deletions(-) 
>  > >  
>  > > diff --git a/drivers/staging/gdm72xx/usb_ids.h 
> b/drivers/staging/gdm72xx/usb_ids.h 
>  > > index 8ce544d..2b50ac6 100644 
>  > > --- a/drivers/staging/gdm72xx/usb_ids.h 
>  > > +++ b/drivers/staging/gdm72xx/usb_ids.h 
>  > > @@ -32,8 +32,8 @@ 
>  > >  #define BL_PID_MASK0xffc0 
>  > >   
>  > >  #define USB_DEVICE_BOOTLOADER(vid, pid)\ 
>  > > -{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},\ 
>  > > -{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)} 
>  > > +({USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},\ 
>  > > +{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)}) 
>  >  
>  > checkpatch isn't always correct.  This is one such example. 
>  >  
>  > Does this even compile? 
>  > 
> 
> Yes. It did. I compiled the module against 3.2.0-4-amd64 from Debian and 
> 4.0.0 vanilla from kernel.org. One thing that I don't understand is this:
> 
> [jaime@hpsylinux staging]$ make -C /lib/modules/3.2.0-4-amd64/build 
> M=$PWD/drivers/staging/gdm72xx/ modules
> make: Entering directory `/usr/src/linux-headers-3.2.0-4-amd64'
>   Building modules, stage 2.
>   MODPOST 0 modules

That implies you didn't select the driver to be built in your .config
file.  Are you sure you did that?

> make: Leaving directory `/usr/src/linux-headers-3.2.0-4-amd64'
> [jaime@hpsylinux staging]$ make -C /lib/modules/4.0.0/build 
> M=$PWD/drivers/staging/gdm72xx/ modules
> make: Entering directory `/home/jaime/Pprojects/linux_kernel/linux-4.0'
>   Building modules, stage 2.
>   MODPOST 0 modules
> make: Leaving directory `/home/jaime/Pprojects/linux_kernel/linux-4.0'
> 
> MODPOST 0 modules? I get the same result without making the patch changes. To 
> resolve this I'll test this on another machine.
> 
> I compiled the whole 4.0.0 kernel and got the image with no problems after 
> making the patch changes.

if you touch the file, and do a 'make -j8' does the file rebuild?  If
not, then you didn't select the config option.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/3] staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()

[Hartley Sweeten]

On Tuesday, April 21, 2015 6:35 AM, Dan Carpenter wrote:
> On Tue, Apr 21, 2015 at 01:52:09PM +0100, Ian Abbott wrote:
>>>Is that really an improvement?  The original code was actually defined.
>>>
>>>1U << 32 is actually defined.  It is zero.  Which works for us.
>> 
>> According to the C standards it is undefined (for 32-bit unsigned
>> int).  See the late draft of ISO/IEC 9899:2011 (dubbed C11) at
>> , §6.5.7,
>> Semantics:
>> 
>> 3. The integer promotions are performed on each of the operands. The
>> type of the result is that of the promoted left operand.If the value
>> of the right operand is negative or is greater than or equal to the
>> width of the promoted left operand, the behavior is undefined.
>>
>
> Yeah.  You're right.
>
>> 
>> It would be better to use (1U << b_chans) instead of (1 << b_chans)
>> in the code above, or possibly the slightly more obtuse:
>> 
>>  b_mask = ((b_chans < 32) ? 1 << b_chans : 0) - 1;
>
> I like just switching Hartley's 1 to 1U.

A quick test:

#include 

int main(int argc, char *argv[])
{
int shift;

shift = 31;
printf("(1 << %d) - 1 = %x\n", shift, (1 << shift) - 1);
printf("(1U << %d) - 1 = %x\n", shift, (1U << shift) - 1);

shift = 32;
printf("(1 << %d) - 1 = %x\n", shift, (1 << shift) - 1);
printf("(1U << %d) - 1 = %x\n", shift, (1U << shift) - 1);

return 0;
}

Produces this result:

(1 << 31) - 1 = 7fff
(1U << 31) - 1 = 7fff
(1 << 32) - 1 = 0
(1U << 32) - 1 = 0

Looks like the 1 vs 1U doesn't make any difference. And a shift of 32
results in the wrong value.

Are you ok with the original patch?

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/3] Drivers: hv: vmbus: additional fixes for the setup/teardown path

[Vitaly Kuznetsov]

Changes since v1 (PATCH 1/3):
- Return -EAGAIN instead of open_info->response.open_result.status [Dexuan Cui]
- Avoid 'if (ret != 0)' statement [Dan Carpenter] 

Original description:
K. Y.,

Here are 3 additional patches for your "[PATCH 0/5] Drivers: hv: vmbus: Cleanup
the vmbus unload path" series (with the fix I suggested). I tested the whole
set on Gen2 Win2012R2 guest, load/unload path seems being stable. Can you
please take a look?

Thanks,

Vitaly Kuznetsov (3):
  Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths
  Drivers: hv: vmbus: kill tasklets on module unload
  Drivers: hv: vmbus: setup irq after synic is initialized

 drivers/hv/channel.c   | 13 ++---
 drivers/hv/vmbus_drv.c | 10 +++---
 2 files changed, 13 insertions(+), 10 deletions(-)

-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/3] Drivers: hv: vmbus: kill tasklets on module unload

[Vitaly Kuznetsov]

Explicitly kill tasklets we create on module unload.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/vmbus_drv.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 2b56260..cf20400 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1108,6 +1108,7 @@ static void __exit vmbus_exit(void)
hv_synic_clockevents_cleanup();
vmbus_disconnect();
hv_remove_vmbus_irq();
+   tasklet_kill(&msg_dpc);
vmbus_free_channels();
if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
atomic_notifier_chain_unregister(&panic_notifier_list,
@@ -1115,8 +1116,10 @@ static void __exit vmbus_exit(void)
}
bus_unregister(&hv_bus);
hv_cleanup();
-   for_each_online_cpu(cpu)
+   for_each_online_cpu(cpu) {
+   tasklet_kill(hv_context.event_dpc[cpu]);
smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
+   }
acpi_bus_unregister_driver(&vmbus_acpi_driver);
hv_cpu_hotplug_quirk(false);
 }
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() failure paths

[Vitaly Kuznetsov]

In case there was an error reported in the response to the 
CHANNELMSG_OPENCHANNEL
call we need to do the cleanup as a vmbus_open() user won't be doing it after
receiving an error. The cleanup should be done on all failure paths. We also 
need
to avoid returning open_info->response.open_result.status as the return value as
all other errors we return from vmbus_open() are -EXXX and vmbus_open() callers
are not supposed to analyze host error codes.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/channel.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 54da66d..7a1c2db 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -178,19 +178,18 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 
send_ringbuffer_size,
goto error1;
}
 
-
-   if (open_info->response.open_result.status)
-   err = open_info->response.open_result.status;
-
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
list_del(&open_info->msglistentry);
spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
-   if (err == 0)
-   newchannel->state = CHANNEL_OPENED_STATE;
+   if (open_info->response.open_result.status) {
+   err = -EAGAIN;
+   goto error_gpadl;
+   }
 
+   newchannel->state = CHANNEL_OPENED_STATE;
kfree(open_info);
-   return err;
+   return 0;
 
 error1:
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/3] Drivers: hv: vmbus: setup irq after synic is initialized

[Vitaly Kuznetsov]

vmbus_isr() uses synic pages which are being setup in hv_synic_alloc(), we
need to register this irq handler only after we allocate all the required
pages.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/vmbus_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index cf20400..e922804 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -822,11 +822,12 @@ static int vmbus_bus_init(int irq)
if (ret)
goto err_cleanup;
 
-   hv_setup_vmbus_irq(vmbus_isr);
-
ret = hv_synic_alloc();
if (ret)
goto err_alloc;
+
+   hv_setup_vmbus_irq(vmbus_isr);
+
/*
 * Initialize the per-cpu interrupt state and
 * connect to the host.
-- 
1.9.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 0/2] staging: comedi: hide subdevice runflags stuff

[Hartley Sweeten]

On Tuesday, April 21, 2015 5:18 AM, Ian Abbott wrote:
> Keep the details of the comedi subdevice `runflags` member local to
> "comedi_fops.c".  In particular, the usage of the
> `COMEDI_SRF_FREE_SPRIV` run-flag doesn't really fit in all that well
> with the others.  It's used as a marker to indicate the subdevice's
> `private` pointer can be automatically freed by the subdevice
> clean-up code, whereas the others are associated with the operation of
> asynchronous comedi commands.  Abstract it's usage away in a couple of
> new wrapper functions.
>
> 1) staging: comedi: wrap COMEDI_SRF_FREE_SPRIV usage
> 2) staging: comedi: move COMEDI_SRF_... macros to "comedi_fops.c"
>
>  drivers/staging/comedi/comedi_fops.c   | 41 
> --
>  drivers/staging/comedi/comedi_internal.h   |  1 +
>  drivers/staging/comedi/comedidev.h | 18 +-
>  drivers/staging/comedi/drivers.c   |  2 +-
>  .../staging/comedi/drivers/amplc_dio200_common.c   |  6 ++--
>  5 files changed, 45 insertions(+), 23 deletions(-)

Reviewed-by: H Hartley Sweeten 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()

[Ian Abbott]

On 21/04/15 17:07, Hartley Sweeten wrote:

On Tuesday, April 21, 2015 6:35 AM, Dan Carpenter wrote:

On Tue, Apr 21, 2015 at 01:52:09PM +0100, Ian Abbott wrote:

Is that really an improvement?  The original code was actually defined.

1U << 32 is actually defined.  It is zero.  Which works for us.


According to the C standards it is undefined (for 32-bit unsigned
int).  See the late draft of ISO/IEC 9899:2011 (dubbed C11) at
, §6.5.7,
Semantics:

3. The integer promotions are performed on each of the operands. The
type of the result is that of the promoted left operand.If the value
of the right operand is negative or is greater than or equal to the
width of the promoted left operand, the behavior is undefined.



Yeah.  You're right.



It would be better to use (1U << b_chans) instead of (1 << b_chans)
in the code above, or possibly the slightly more obtuse:

b_mask = ((b_chans < 32) ? 1 << b_chans : 0) - 1;


I like just switching Hartley's 1 to 1U.


A quick test:

#include 

int main(int argc, char *argv[])
{
 int shift;

 shift = 31;
 printf("(1 << %d) - 1 = %x\n", shift, (1 << shift) - 1);
 printf("(1U << %d) - 1 = %x\n", shift, (1U << shift) - 1);

 shift = 32;
 printf("(1 << %d) - 1 = %x\n", shift, (1 << shift) - 1);
 printf("(1U << %d) - 1 = %x\n", shift, (1U << shift) - 1);

 return 0;
}

Produces this result:

(1 << 31) - 1 = 7fff
(1U << 31) - 1 = 7fff
(1 << 32) - 1 = 0
(1U << 32) - 1 = 0

Looks like the 1 vs 1U doesn't make any difference. And a shift of 32
results in the wrong value.

Are you ok with the original patch?


Yes, but 1U is cleaner.

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Moving comedi to mainline soon?

[Joe Perches]

It seems that most all of the comedi code cleanliness
issues have been resolved.

What's left before this is accepted into mainline?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] staging: comedi: comedi_bond: fix 'b_mask' calc in bonding_dio_insn_bits()

[Dan Carpenter]

I knew the original was undefined because Ian showed me the relevant
section from the standard.  I'm actually surprised that it doesn't work
in GCC.  Using -fno-strict-overflow doesn't help either.  I think at the
optimizations that the kernel uses -O2 and -Os the original "works".

Anyway, the old code is definitely wrong.

But the new code is also undefined because we are subtracting from
INT_MIN.  I imagine how GCC could handle the undefined behavior in an
unexpected way so the new code is probably fine.  But we may as well
just be pedantic.

b_mask = (b_chans < 32) ? ((1U << b_chans) - 1)
: 0x;

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" 

The same area used for ioremap() is used for the MTRR area.
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Sudip Mukherjee 
Cc: Teddy Wang 
Cc: Greg Kroah-Hartman 
Cc: Suresh Siddha 
Cc: Ingo Molnar 
Cc: Thomas Gleixner 
Cc: Juergen Gross 
Cc: Daniel Vetter 
Cc: Andy Lutomirski 
Cc: Dave Airlie 
Cc: Antonino Daplas 
Cc: Jean-Christophe Plagniol-Villard 
Cc: Tomi Valkeinen 
Cc: de...@driverdev.osuosl.org
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Luis R. Rodriguez 
---
 drivers/staging/sm750fb/sm750.c| 36 
 drivers/staging/sm750fb/sm750.h|  3 ---
 drivers/staging/sm750fb/sm750_hw.c |  3 +--
 3 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3c7ea95..cf57e3e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -16,9 +16,6 @@
 #include
 #include
 #include 
-#ifdef CONFIG_MTRR
-#include 
-#endif
 #include 
 #include "sm750.h"
 #include "sm750_hw.h"
@@ -47,9 +44,7 @@ typedef int (*PROC_SPEC_INITHW)(struct lynx_share*, struct 
pci_dev*);
 /* common var for all device */
 static int g_hwcursor = 1;
 static int g_noaccel;
-#ifdef CONFIG_MTRR
 static int g_nomtrr;
-#endif
 static const char *g_fbmode[] = {NULL, NULL};
 static const char *g_def_fbmode = "800x600-16@60";
 static char *g_settings = NULL;
@@ -1126,11 +1121,8 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
 
pr_info("share->revid = %02x\n", share->revid);
share->pdev = pdev;
-#ifdef CONFIG_MTRR
share->mtrr_off = g_nomtrr;
share->mtrr.vram = 0;
-   share->mtrr.vram_added = 0;
-#endif
share->accel_off = g_noaccel;
share->dual = g_dualview;
spin_lock_init(&share->slock);
@@ -1158,22 +1150,9 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
goto err_map;
}
 
-#ifdef CONFIG_MTRR
-   if (!share->mtrr_off) {
-   pr_info("enable mtrr\n");
-   share->mtrr.vram = mtrr_add(share->vidmem_start,
-   share->vidmem_size,
-   MTRR_TYPE_WRCOMB, 1);
-
-   if (share->mtrr.vram < 0) {
-   /* don't block driver with the failure of MTRR */
-   pr_err("Unable to setup MTRR.\n");
-   } else {
-   share->mtrr.vram_added = 1;
-   pr_info("MTRR added succesfully\n");
-   }
-   }
-#endif
+   if (!share->mtrr_off)
+   share->mtrr.vram = arch_phys_wc_add(share->vidmem_start,
+   share->vidmem_size);
 
memset_io(share->pvMem, 0, share->vidmem_size);
 
@@ -1274,12 +1253,7 @@ static void __exit lynxfb_pci_remove(struct pci_dev 
*pdev)
/* release frame buffer */
framebuffer_relea

[PATCH v3] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" 

The same area used for ioremap() is used for the MTRR area.
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Sudip Mukherjee 
Cc: Teddy Wang 
Cc: Greg Kroah-Hartman 
Cc: Suresh Siddha 
Cc: Ingo Molnar 
Cc: Thomas Gleixner 
Cc: Juergen Gross 
Cc: Daniel Vetter 
Cc: Andy Lutomirski 
Cc: Dave Airlie 
Cc: Antonino Daplas 
Cc: Jean-Christophe Plagniol-Villard 
Cc: Tomi Valkeinen 
Cc: de...@driverdev.osuosl.org
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Luis R. Rodriguez 
---
 drivers/staging/sm750fb/sm750.c| 36 
 drivers/staging/sm750fb/sm750.h|  3 ---
 drivers/staging/sm750fb/sm750_hw.c |  3 +--
 3 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3c7ea95..cf57e3e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -16,9 +16,6 @@
 #include
 #include
 #include 
-#ifdef CONFIG_MTRR
-#include 
-#endif
 #include 
 #include "sm750.h"
 #include "sm750_hw.h"
@@ -47,9 +44,7 @@ typedef int (*PROC_SPEC_INITHW)(struct lynx_share*, struct 
pci_dev*);
 /* common var for all device */
 static int g_hwcursor = 1;
 static int g_noaccel;
-#ifdef CONFIG_MTRR
 static int g_nomtrr;
-#endif
 static const char *g_fbmode[] = {NULL, NULL};
 static const char *g_def_fbmode = "800x600-16@60";
 static char *g_settings = NULL;
@@ -1126,11 +1121,8 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
 
pr_info("share->revid = %02x\n", share->revid);
share->pdev = pdev;
-#ifdef CONFIG_MTRR
share->mtrr_off = g_nomtrr;
share->mtrr.vram = 0;
-   share->mtrr.vram_added = 0;
-#endif
share->accel_off = g_noaccel;
share->dual = g_dualview;
spin_lock_init(&share->slock);
@@ -1158,22 +1150,9 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
goto err_map;
}
 
-#ifdef CONFIG_MTRR
-   if (!share->mtrr_off) {
-   pr_info("enable mtrr\n");
-   share->mtrr.vram = mtrr_add(share->vidmem_start,
-   share->vidmem_size,
-   MTRR_TYPE_WRCOMB, 1);
-
-   if (share->mtrr.vram < 0) {
-   /* don't block driver with the failure of MTRR */
-   pr_err("Unable to setup MTRR.\n");
-   } else {
-   share->mtrr.vram_added = 1;
-   pr_info("MTRR added succesfully\n");
-   }
-   }
-#endif
+   if (!share->mtrr_off)
+   share->mtrr.vram = arch_phys_wc_add(share->vidmem_start,
+   share->vidmem_size);
 
memset_io(share->pvMem, 0, share->vidmem_size);
 
@@ -1274,12 +1253,7 @@ static void __exit lynxfb_pci_remove(struct pci_dev 
*pdev)
/* release frame buffer */
framebuffer_relea

[PATCH net-next, 1/1] hv_netvsc: call dump_rndis_message() only in netvsc debug mode

[sixiao]

From: Simon Xiao 

Signed-off-by: Simon Xiao 
Reviewed-by: K. Y. Srinivasan 
Reviewed-by: Haiyang Zhang 
---
 drivers/net/hyperv/hyperv_net.h   | 3 +++
 drivers/net/hyperv/netvsc_drv.c   | 8 
 drivers/net/hyperv/rndis_filter.c | 3 ++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index a10b316..c9be35e 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -28,6 +28,9 @@
 #include 
 #include 
 
+/* flag for netvsc debug mode */
+extern int debug_mode;
+
 /* RSS related */
 #define OID_GEN_RECEIVE_SCALE_CAPABILITIES 0x00010203  /* query only */
 #define OID_GEN_RECEIVE_SCALE_PARAMETERS 0x00010204  /* query and set */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a3a9d38..7c41864 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -52,6 +52,10 @@ static int ring_size = 128;
 module_param(ring_size, int, S_IRUGO);
 MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)");
 
+int debug_mode = 0;
+module_param(debug_mode, int, S_IRUGO);
+MODULE_PARM_DESC(debug_mode, "debug mode: zero(0) for non-debug mode; non-zero 
for debug mode");
+
 static void do_set_multicast(struct work_struct *w)
 {
struct net_device_context *ndevctx =
@@ -999,6 +1003,10 @@ static int __init netvsc_drv_init(void)
pr_info("Increased ring_size to %d (min allowed)\n",
ring_size);
}
+
+   if (debug_mode != 0)
+   pr_info("Run netvsc in debug mode");
+
return vmbus_driver_register(&netvsc_drv);
 }
 
diff --git a/drivers/net/hyperv/rndis_filter.c 
b/drivers/net/hyperv/rndis_filter.c
index 0d92efe..a3f43f6 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -429,7 +429,8 @@ int rndis_filter_receive(struct hv_device *dev,
 
rndis_msg = pkt->data;
 
-   dump_rndis_message(dev, rndis_msg);
+   if (debug_mode != 0)
+   dump_rndis_message(dev, rndis_msg);
 
switch (rndis_msg->ndis_msg_type) {
case RNDIS_MSG_PACKET:
-- 
1.8.5.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next,1/1] hv_netvsc: call dump_rndis_message() only in netvsc debug mode

[David Miller]

From: six...@microsoft.com
Date: Tue, 21 Apr 2015 15:58:05 -0700

> From: Simon Xiao 
> 
> Signed-off-by: Simon Xiao 
> Reviewed-by: K. Y. Srinivasan 
> Reviewed-by: Haiyang Zhang 

I just gave you feedback on this patch in response to your
original submission, do not ignore it.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH net-next, 1/1] hv_netvsc: call dump_rndis_message() only in netvsc debug mode

[Simon Xiao]

> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Tuesday, April 21, 2015 2:49 PM
> To: Simon Xiao
> Cc: KY Srinivasan; Haiyang Zhang; de...@linuxdriverproject.org;
> net...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH net-next,1/1] hv_netvsc: call dump_rndis_message() only in
> netvsc debug mode
> 
> From: six...@microsoft.com
> Date: Tue, 21 Apr 2015 15:58:05 -0700
> 
> > From: Simon Xiao 
> >
> > Signed-off-by: Simon Xiao 
> > Reviewed-by: K. Y. Srinivasan 
> > Reviewed-by: Haiyang Zhang 
> 
> I just gave you feedback on this patch in response to your original 
> submission,
> do not ignore it.

Thanks for your feedback, David.

In current netvsc driver, for each packet received, it will call 
dump_rndis_message() 
to try to dump the rndis packet information by netdev_dbg(). 
In non-debug mode, dump_rndis_message() will not dump anything 
but it still initialize some local variables and process the switch logic in 
the function 
of dump_rndis_message(), which is unnecessary, especially in high network 
throughput situation.

My change is to have a run-time config flag to control the execution of 
dump_rndis_message() 
and avoid above unnecessary cost in non-debug mode.
In the default case, it will be non-debug mode,
 and rndis_filter_receive() will not call dump_rndis_message() 
which saves the above extra cost for each packet received.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next,1/1] hv_netvsc: call dump_rndis_message() only in netvsc debug mode

[David Miller]

From: Simon Xiao 
Date: Tue, 21 Apr 2015 22:14:14 +

> In current netvsc driver, for each packet received, it will call
> dump_rndis_message() to try to dump the rndis packet information by
> netdev_dbg().  In non-debug mode, dump_rndis_message() will not dump
> anything but it still initialize some local variables and process
> the switch logic in the function of dump_rndis_message(), which is
> unnecessary, especially in high network throughput situation.

See NETIF_MSG_* and use it properly in your driver, read other drivers
and learn how to properly use it for things like this.

I'm not going to explain this a third time.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Warning: message 1YiPTz-0007Bh-GG delayed 48 hours

[Mail Delivery System]

This message was created automatically by mail delivery software.
A message that you sent has not yet been delivered to one or more of its
recipients after more than 48 hours on the queue on server.g1novelas.org.

The message identifier is: 1YiPTz-0007Bh-GG
The subject of the message is: Êëèåíòñêèå áàçû +79133913837 Óçíàéòå 
ïîäðîáíåå!!!<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= 
<=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= <=<= 
<=<= <=<= <=<= <=
The date of the message is:Wed, 15 Apr 2015 21:40:40 +0600

The address to which the message has not yet been delivered is:

  de...@driverdev.osuosl.org
Delay reason: SMTP error from remote mail server after RCPT 
TO::
host smtp2.osuosl.org [140.211.166.133]: 451 4.7.1 
:
Recipient address rejected: Greylisted for 5 minutes

No action is required on your part. Delivery attempts will continue for
some time, and this warning may be repeated at intervals if the message
remains undelivered. Eventually the mail delivery software will give up,
and when that happens, the message will be returned to you.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

vnc over ssh with tp-link tl-wn725n causes laptop to freeze

[dborlau...@gmail.com]

Dear All,

Thank you kindly in advance for your help.  Please forgive any trivial
questions or mistakes by me.  I am new to linux (1 month).  Hope this is
the proper place for such a question.  Here goes.

I have a number of tp-link tl-wn725n usb adapters that I want to use in
laptops.  The laptops are running debian wheezy v.7.8 fully updated.  I
can use the laptops with the tp-link adapters fine for web surfing,
downloading a file, etc..  For such things my laptop tp-link combos work
great, everything is fine.

I installed the tp-link adapters via:
http://brilliantlyeasy.com/ubuntu-linux-tl-wn725n-tp-link-version-2-wifi-driver-install/

using the following commands:
apt-get update
apt-get install linux-headers-$(uname -r)
apt-get update
apt-get install build-essential
apt-get install git
git clone https://github.com/lwfinger/rtl8188eu
cd rtl8188eu
make all
make install
insmod 8188eu.ko

The problem is that when I attempt to vnc over an ssh connection to a
debian box running either x11vnc or tightvncserver, I can make the
connection, but then the laptop is freezes.  This freeze happens withing
2 seconds of making the connection.  When the laptop freezes the adapter
lights go dark.  The laptop becomes unresponsive, from what I can tell,
and I have to do a hard reboot.  I have tried vnc over ssh with same
laptop over the ethernet nic with the tp-link wireless removed from the
laptop and I have no problems.  I have also tried another laptop, same
behavior, vnc over ssh works with wired, freezes laptop with tp-link.
laptops are lenovo x120e and z40-70, if this helps.

Hopefully I have provided some good information that helps.  Please let
me know if there is something I've done wrong, such as missed the latest
driver, etc.  Happy to try to get this working and report back.

Thank you kindly,
anon



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: sm750: Fix the Makefile option error

[Binbin Zhou]

The sm750fb's Kconfig option is CONFIG_FB_SM750, not CONFIG_FB_SM7XX.
Thus fix it to make the sm750fb can be built successfully.

Cc: Sudip Mukherjee 
Signed-off-by: Binbin Zhou 
Reviewed-by: Huacai Chen 
---
 drivers/staging/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 2bbd1bf..a12221f 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -23,7 +23,7 @@ obj-$(CONFIG_VT6656)  += vt6656/
 obj-$(CONFIG_VME_BUS)  += vme/
 obj-$(CONFIG_IIO)  += iio/
 obj-$(CONFIG_FB_SM7XX) += sm7xxfb/
-obj-$(CONFIG_FB_SM7XX) += sm750fb/
+obj-$(CONFIG_FB_SM750) += sm750fb/
 obj-$(CONFIG_FB_XGI)   += xgifb/
 obj-$(CONFIG_USB_EMXX) += emxx_udc/
 obj-$(CONFIG_FT1000)   += ft1000/
-- 
1.9.0




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] vfs: export symbol 'getname' and 'putname'

[Boqun Feng]

getname/putname in fs/namei.c is a well-implemented way to copy a file
name from userland, however other ways, such as directly calling
__getname() and strncpy_from_user(), may lack features(e.g. auditing and
reusing), introduce errors or at least reinvent wheels. Therefore for
places need a kernel file name copy from userland, it's better to use
getname and putname if possible.

To be able to use these functions all over the kernel, symbols 'getname'
and 'putname' are exported and comments of their behaviors and
constraints are added.

Suggested-by: Al Viro 
Signed-off-by: Boqun Feng 
---
 fs/namei.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index c83145a..472911c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -199,11 +199,23 @@ error:
return err;
 }
 
+/**
+ * getname() - Get a file name copy from userland
+ * @filename: userland pointer to the file name
+ *
+ * If successful, return a 'struct filename' pointer and ->name is the pointer
+ * to the kernel copy of the file name, otherwise an ERR_PTR.
+ *
+ * getname() should only be called in a system call context, and for each
+ * getname() that returns a successful value, callers must ensure exactly one
+ * corresponding putname() is called before returning to userland.
+ */
 struct filename *
 getname(const char __user * filename)
 {
return getname_flags(filename, 0, NULL);
 }
+EXPORT_SYMBOL(getname);
 
 struct filename *
 getname_kernel(const char * filename)
@@ -242,6 +254,11 @@ getname_kernel(const char * filename)
return result;
 }
 
+/* putname() - Release a 'struct filename' structure
+ * @name: the 'struct filename' structure to be release
+ *
+ * See more at getname()
+ */
 void putname(struct filename *name)
 {
BUG_ON(name->refcnt <= 0);
@@ -255,6 +272,7 @@ void putname(struct filename *name)
} else
__putname(name);
 }
+EXPORT_SYMBOL(putname);
 
 static int check_acl(struct inode *inode, int mask)
 {
-- 
2.3.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/2] staging: lustre: Replace ll_getname with vfs' getname

[Boqun Feng]

As Al Viro pointed out:

https://lkml.org/lkml/2015/4/11/243

There are bugs in ll_getname() because of wrong assumptions of returning
values from strncpy_from_user(). Moreover, what ll_getname want to do is
just to try copy the file name from userland. Since we already have
getname() for the same purpose, it's better to replace ll_getname() with
getname().

To do that, we need to:
1) export the symbols of getname() and putname() to be used by modules.
2) actually replace ll_getname()/ll_putname() with getname()/putname().

One more thing is that as ll_getname() and getname() both treat a zero-length
file name as an error(-ENOENT), and if ll_getname() or getname() has an error,
ll_dir_ioctl() will return the error immediately, so checking whether these 
names
are zero-length is unnecessary and -ENIVAL shall not return from that code path,
no matter using ll_getname() or getname(). So remove the checking code. 

This patchset is based on v4.0, and I only did build tests, because I found a
little difficult to set up a lustre environment. I'll try to do the testing once
I'm able to set up an environment, in the meanwhile, comments,
inputs from lustre's point of view and voluntary tests are welcome. Thank you. 
;-)

Regards,
Boqun Feng


Boqun Feng (2):
  vfs: Export symbol 'getname' and 'putname'
  staging: lustre: replace ll_{get,put}name() with {get,put}name()

 drivers/staging/lustre/lustre/llite/dir.c  | 60 ++
 .../staging/lustre/lustre/llite/llite_internal.h   |  2 +-
 drivers/staging/lustre/lustre/llite/namei.c|  2 +-
 fs/namei.c | 18 +++
 4 files changed, 36 insertions(+), 46 deletions(-)

-- 
2.3.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] staging: lustre: replace ll_{get, put}name() with {get, put}name()

[Boqun Feng]

As pointed by Al Viro:

https://lkml.org/lkml/2015/4/11/243

There are bugs in ll_getname() because of wrong assumptions of returning
values from strncpy_from_user(). Moreover, what ll_getname want to do is
just to try copy the file name from userland. Since we already have
getname() for the same purpose, it's better to replace ll_getname() with
getname(), so is ll_putname().

Besides, remove unused code for checking whether namelen is 0 or not in
case LL_IOC_REMOVE_ENTRY, because zero-length file name is already
handled by getname() in the same way as ll_getname().

Suggested-by: Al Viro 
Signed-off-by: Boqun Feng 
---
 drivers/staging/lustre/lustre/llite/dir.c  | 60 ++
 .../staging/lustre/lustre/llite/llite_internal.h   |  2 +-
 drivers/staging/lustre/lustre/llite/namei.c|  2 +-
 3 files changed, 18 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/dir.c 
b/drivers/staging/lustre/lustre/llite/dir.c
index a182019..c75fc38 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -1216,30 +1216,6 @@ out:
return rc;
 }
 
-static char *
-ll_getname(const char __user *filename)
-{
-   int ret = 0, len;
-   char *tmp = __getname();
-
-   if (!tmp)
-   return ERR_PTR(-ENOMEM);
-
-   len = strncpy_from_user(tmp, filename, PATH_MAX);
-   if (len == 0)
-   ret = -ENOENT;
-   else if (len > PATH_MAX)
-   ret = -ENAMETOOLONG;
-
-   if (ret) {
-   __putname(tmp);
-   tmp =  ERR_PTR(ret);
-   }
-   return tmp;
-}
-
-#define ll_putname(filename) __putname(filename)
-
 static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned long 
arg)
 {
struct inode *inode = file_inode(file);
@@ -1441,7 +1417,7 @@ free_lmv:
return rc;
}
case LL_IOC_REMOVE_ENTRY: {
-   char*filename = NULL;
+   struct filename *name = NULL;
int  namelen = 0;
int  rc;
 
@@ -1453,20 +1429,16 @@ free_lmv:
if (!(exp_connect_flags(sbi->ll_md_exp) & OBD_CONNECT_LVB_TYPE))
return -ENOTSUPP;
 
-   filename = ll_getname((const char *)arg);
-   if (IS_ERR(filename))
-   return PTR_ERR(filename);
+   name = getname((const char *)arg);
+   if (IS_ERR(name))
+   return PTR_ERR(name);
 
-   namelen = strlen(filename);
-   if (namelen < 1) {
-   rc = -EINVAL;
-   goto out_rmdir;
-   }
+   namelen = strlen(name->name);
+
+   rc = ll_rmdir_entry(inode, name->name, namelen);
 
-   rc = ll_rmdir_entry(inode, filename, namelen);
-out_rmdir:
-   if (filename)
-   ll_putname(filename);
+   if (name)
+   putname(name);
return rc;
}
case LL_IOC_LOV_SWAP_LAYOUTS:
@@ -1481,16 +1453,16 @@ out_rmdir:
struct lov_user_md *lump;
struct lov_mds_md *lmm = NULL;
struct mdt_body *body;
-   char *filename = NULL;
+   struct filename *name = NULL;
int lmmsize;
 
if (cmd == IOC_MDC_GETFILEINFO ||
cmd == IOC_MDC_GETFILESTRIPE) {
-   filename = ll_getname((const char *)arg);
-   if (IS_ERR(filename))
-   return PTR_ERR(filename);
+   name = getname((const char *)arg);
+   if (IS_ERR(name))
+   return PTR_ERR(name);
 
-   rc = ll_lov_getstripe_ea_info(inode, filename, &lmm,
+   rc = ll_lov_getstripe_ea_info(inode, name->name, &lmm,
  &lmmsize, &request);
} else {
rc = ll_dir_getstripe(inode, &lmm, &lmmsize, &request);
@@ -1556,8 +1528,8 @@ skip_lmm:
 
 out_req:
ptlrpc_req_finished(request);
-   if (filename)
-   ll_putname(filename);
+   if (name)
+   putname(name);
return rc;
}
case IOC_LOV_GETINFO: {
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h 
b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 2af1d72..0950565 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -714,7 +714,7 @@ struct inode *ll_iget(struct super_block *sb, ino_t hash,
 int ll_md_blocking_ast(struct ldlm_lock *, struct ldlm_lock_desc *,
   void *data, int flag);
 struct dentry *ll_splice_alias(struct inode *inode, st

Re: [PATCH] staging: sm750: Fix the Makefile option error

[Sudip Mukherjee]

On Wed, Apr 22, 2015 at 11:37:26AM +0800, Binbin Zhou wrote:
> The sm750fb's Kconfig option is CONFIG_FB_SM750, not CONFIG_FB_SM7XX.
> Thus fix it to make the sm750fb can be built successfully.

I always used to build sm750fb and sm7xxfb together, so failed to notice
the error. thanks.

Acked-by: Sudip Mukherjee 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging: lustre: replace ll_{get,put}name() with {get,put}name()

[Dilger, Andreas]

On 2015/04/21, 9:50 PM, "Boqun Feng"  wrote:

>As pointed by Al Viro:
>
>https://lkml.org/lkml/2015/4/11/243
>
>There are bugs in ll_getname() because of wrong assumptions of returning
>values from strncpy_from_user(). Moreover, what ll_getname want to do is
>just to try copy the file name from userland. Since we already have
>getname() for the same purpose, it's better to replace ll_getname() with
>getname(), so is ll_putname().
>
>Besides, remove unused code for checking whether namelen is 0 or not in
>case LL_IOC_REMOVE_ENTRY, because zero-length file name is already
>handled by getname() in the same way as ll_getname().
>
>Suggested-by: Al Viro 
>Signed-off-by: Boqun Feng 

Looks good, you can add my:
Reviewed-by: Andreas Dilger 

>---
> drivers/staging/lustre/lustre/llite/dir.c  | 60
>++
> .../staging/lustre/lustre/llite/llite_internal.h   |  2 +-
> drivers/staging/lustre/lustre/llite/namei.c|  2 +-
> 3 files changed, 18 insertions(+), 46 deletions(-)
>
>diff --git a/drivers/staging/lustre/lustre/llite/dir.c
>b/drivers/staging/lustre/lustre/llite/dir.c
>index a182019..c75fc38 100644
>--- a/drivers/staging/lustre/lustre/llite/dir.c
>+++ b/drivers/staging/lustre/lustre/llite/dir.c
>@@ -1216,30 +1216,6 @@ out:
>   return rc;
> }
> 
>-static char *
>-ll_getname(const char __user *filename)
>-{
>-  int ret = 0, len;
>-  char *tmp = __getname();
>-
>-  if (!tmp)
>-  return ERR_PTR(-ENOMEM);
>-
>-  len = strncpy_from_user(tmp, filename, PATH_MAX);
>-  if (len == 0)
>-  ret = -ENOENT;
>-  else if (len > PATH_MAX)
>-  ret = -ENAMETOOLONG;
>-
>-  if (ret) {
>-  __putname(tmp);
>-  tmp =  ERR_PTR(ret);
>-  }
>-  return tmp;
>-}
>-
>-#define ll_putname(filename) __putname(filename)
>-
> static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned
>long arg)
> {
>   struct inode *inode = file_inode(file);
>@@ -1441,7 +1417,7 @@ free_lmv:
>   return rc;
>   }
>   case LL_IOC_REMOVE_ENTRY: {
>-  char*filename = NULL;
>+  struct filename *name = NULL;
>   int  namelen = 0;
>   int  rc;
> 
>@@ -1453,20 +1429,16 @@ free_lmv:
>   if (!(exp_connect_flags(sbi->ll_md_exp) & OBD_CONNECT_LVB_TYPE))
>   return -ENOTSUPP;
> 
>-  filename = ll_getname((const char *)arg);
>-  if (IS_ERR(filename))
>-  return PTR_ERR(filename);
>+  name = getname((const char *)arg);
>+  if (IS_ERR(name))
>+  return PTR_ERR(name);
> 
>-  namelen = strlen(filename);
>-  if (namelen < 1) {
>-  rc = -EINVAL;
>-  goto out_rmdir;
>-  }
>+  namelen = strlen(name->name);
>+
>+  rc = ll_rmdir_entry(inode, name->name, namelen);
> 
>-  rc = ll_rmdir_entry(inode, filename, namelen);
>-out_rmdir:
>-  if (filename)
>-  ll_putname(filename);
>+  if (name)
>+  putname(name);
>   return rc;
>   }
>   case LL_IOC_LOV_SWAP_LAYOUTS:
>@@ -1481,16 +1453,16 @@ out_rmdir:
>   struct lov_user_md *lump;
>   struct lov_mds_md *lmm = NULL;
>   struct mdt_body *body;
>-  char *filename = NULL;
>+  struct filename *name = NULL;
>   int lmmsize;
> 
>   if (cmd == IOC_MDC_GETFILEINFO ||
>   cmd == IOC_MDC_GETFILESTRIPE) {
>-  filename = ll_getname((const char *)arg);
>-  if (IS_ERR(filename))
>-  return PTR_ERR(filename);
>+  name = getname((const char *)arg);
>+  if (IS_ERR(name))
>+  return PTR_ERR(name);
> 
>-  rc = ll_lov_getstripe_ea_info(inode, filename, &lmm,
>+  rc = ll_lov_getstripe_ea_info(inode, name->name, &lmm,
> &lmmsize, &request);
>   } else {
>   rc = ll_dir_getstripe(inode, &lmm, &lmmsize, &request);
>@@ -1556,8 +1528,8 @@ skip_lmm:
> 
> out_req:
>   ptlrpc_req_finished(request);
>-  if (filename)
>-  ll_putname(filename);
>+  if (name)
>+  putname(name);
>   return rc;
>   }
>   case IOC_LOV_GETINFO: {
>diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h
>b/drivers/staging/lustre/lustre/llite/llite_internal.h
>index 2af1d72..0950565 100644
>--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
>+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
>@@ -714,7 +714,7 @@ struct inode *ll_iget(struct super_block *sb, ino_t
>hash,
> i

Re: [PATCH 1/2] vfs: export symbol 'getname' and 'putname'

[Christoph Hellwig]

Nak on exporting symbols for broken staging code.  Please get rid of
the ioctls looking up path names in horrible ways in the lustre code.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Dear Sir/Madam

[Houzan al-Douri .]

Dear Sir/Madam ,

Salaam

Please my English is poor read carefully my word ok ,yes I am Houzan
al-Douri ,Son of Ezzat Ibrahim al-Duri,the late Iraqi general,vice
president and former deputy of the late Saddam Hussein before the 2003
US-led invasion on Iraq.My father is suffering from leukemia was
killed on 17th of April 2015 during 25 minutes of exchange of gunfire
with a force of pro-government paramilitaries in Hamreen,as you can
confirm in website:(
http://news.yahoo.com/iraq-checks-claims-saddam-deputy-izzat-ibrahim-al-155451649.html
):

Since the death of my father,my life is in target as a result most of
my father's loyalist including my three Elder brothers (Ibrahim
al-Douri,Ahmed al-Douri and Ali al-Douri) is under hiding and secretly
fly oversea Countries due to fears and death threat we face every
minute of the day ,our home town of Dawr is not a safe place.I am
seeking your urgent help because I don't want to experience the same
thing that happened to Four of my late father's nephews who were
captured in January 2004(
http://www.dawn.com/news/347929/izzat-ibrahim-s-four-nephews-arrested),
two months after the arrest of his wife and daughter. I fly now for my
safety to Doha city in Qatar in undisclosed location for security
reasons.

I'm pleading for your urgent help to work with my attorney in Arab
Emirate (Dubai) to receive into your bank account a total sum of
Twenty Eight Million us$ Dollars that was saved in my name to a bank
but for security reasons could not apply directly to receive this
amount.If you have interest to work with my attorney whom all
documents were handed over to legally recover the money through the
help of a foreigner by his suggestion,I will accept to give 30% of the
total sum to reward your assistance ,while 10% goes to my attorney's
professional work and 10% will be use to handle all Expenses.

If you're willing to assist Me,please respond back urgently to
indicate your interest while I furnish you with more details,so you
can contact my attorney to prepare agreement between us for you to
declare in honest your guarantee in assurance to return my share when
you receive the whole money into your bank account,he will also
provide appropriately guide of what is required to proceed.

I await urgently to hear from you.

Thank you
Houzan al-Douri.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] vfs: export symbol 'getname' and 'putname'

[Drokin, Oleg]

On Apr 22, 2015, at 1:53 AM, Christoph Hellwig wrote:

> Nak on exporting symbols for broken staging code.  Please get rid of
> the ioctls looking up path names in horrible ways in the lustre code.

For a reference, is there a good example of a non-horrible way to look up a 
pathname?

Thanks.

Bye,
   Oleg
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] vfs: export symbol 'getname' and 'putname'

[Christoph Hellwig]

On Wed, Apr 22, 2015 at 06:27:11AM +, Drokin, Oleg wrote:
> > Nak on exporting symbols for broken staging code.  Please get rid of
> > the ioctls looking up path names in horrible ways in the lustre code.
> 
> For a reference, is there a good example of a non-horrible way to look up a 
> pathname?

Just dont do it from an ioctl, it's got an fd parameter for a reason.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] vfs: export symbol 'getname' and 'putname'

[Greg Kroah-Hartman]

On Tue, Apr 21, 2015 at 10:53:11PM -0700, Christoph Hellwig wrote:
> Nak on exporting symbols for broken staging code.  Please get rid of
> the ioctls looking up path names in horrible ways in the lustre code.

I agree with Christoph, we shouldn't be doing this.  Let's fix lustre
"properly", which should be possible, right?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: sm750: Fix the Makefile option error

[陈华才]

Hi, Sudip,

We also get a sm750 kernel driver from Silicon Motion 
(http://dev.lemote.com/files/upload/lm/kernel/testing/sm750fb.tar.gz), its 
Version is 1.0.8.

I'm not sure whether your driver (which has merged in staging) is newer than 
ours, but it seems yours is older (Because there are LINUX_VERSION_CODE >= 
KERNEL_VERSION(2,6,10) in your driver, and there are LINUX_VERSION_CODE >= 
KERNEL_VERSION(2,6,28) in our driver).

I think you know more about SM750 than us, so please look at my tarball, and if 
ours is better, please update the kernel driver. Thank you very much.

Huacai
 
-- Original --
From:  "Sudip Mukherjee";
Date:  Wed, Apr 22, 2015 12:59 PM
To:  "Binbin Zhou"; 
Cc:  "Greg Kroah-Hartman"; 
"devel"; "Huacai Chen"; "Fuxin 
Zhang"; 
Subject:  Re: [PATCH] staging: sm750: Fix the Makefile option error

 
On Wed, Apr 22, 2015 at 11:37:26AM +0800, Binbin Zhou wrote:
> The sm750fb's Kconfig option is CONFIG_FB_SM750, not CONFIG_FB_SM7XX.
> Thus fix it to make the sm750fb can be built successfully.

I always used to build sm750fb and sm7xxfb together, so failed to notice
the error. thanks.

Acked-by: Sudip Mukherjee 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] vfs: export symbol 'getname' and 'putname'

[Drokin, Oleg]

On Apr 22, 2015, at 2:31 AM, Greg Kroah-Hartman wrote:

> On Tue, Apr 21, 2015 at 10:53:11PM -0700, Christoph Hellwig wrote:
>> Nak on exporting symbols for broken staging code.  Please get rid of
>> the ioctls looking up path names in horrible ways in the lustre code.
> 
> I agree with Christoph, we shouldn't be doing this.  Let's fix lustre
> "properly", which should be possible, right?

Well, sort of.

For the first ioctl we clearly can do an fd pass and
get info from there (need to also teach the tools about the new ioctl,
so I cannot submit a patch right away, but I'll get on it).

In the second case the usecase is a bit more involved.
It deals with the problem of having directory entry pointing to a non-existing
directory (so basically a name only, but we do know it's supposed to be
a directory, just on another server), so I doubt we can even open it to get
the fd.
Now, perhaps we can just allow regular rmdir to ignore the error and kill
the bogus entry anyway, but there was this thought that we might want to alert
the user there's something more fundamentally broken going on here,
and so they would need to call this certain ioctl called if they just would like
to kill the entry anyway as opposed to calling say fsck to make sure there's
nothing else broken.
I am not entirely sure of this idea myself, but it probably made sense
at some point.
I see there's quite a dislike for this approach, so we can remove it if there's
no better option.

Bye,
Oleg
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] vfs: export symbol 'getname' and 'putname'

[Drokin, Oleg]

On Apr 22, 2015, at 2:31 AM, Christoph Hellwig wrote:

> On Wed, Apr 22, 2015 at 06:27:11AM +, Drokin, Oleg wrote:
>>> Nak on exporting symbols for broken staging code.  Please get rid of
>>> the ioctls looking up path names in horrible ways in the lustre code.
>> 
>> For a reference, is there a good example of a non-horrible way to look up a 
>> pathname?
> 
> Just dont do it from an ioctl, it's got an fd parameter for a reason.

I know this is not going to be a popular opinion with you, but sometimes 
opening a file
is just too expensive. 1 RPC roudntrip to open a file and then another one to 
close it.

Also some files could not be opened (fs corruption).

Anyway, I got your point and there will be a solution. I was just hoping there 
was a way
to do it because what if e.g. I need to create something new, not do something 
with already
existing stuff, certainly there's no way to get an fd from a not yet existing 
fs object.

Bye,
Oleg
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel