Re: [PATCH v2 net-next 0/7] net: speedup netns create/delete time

[Dmitry Torokhov]

On Wed, Dec 13, 2017 at 1:52 PM, Eric Dumazet <eric.duma...@gmail.com> wrote:
> On Wed, 2017-12-13 at 13:43 -0800, Dmitry Torokhov wrote:
>> Hi Eric,
>>
>> On Thu, Oct 19, 2017 at 7:11 AM, Eric Dumazet <eduma...@google.com> wrote:
>> >
>> > On Thu, Oct 19, 2017 at 4:48 AM, Tariq Toukan <tar...@mellanox.com> wrote:
>> > >
>> > > Hi Eric,
>> > >
>> > > I just wanted to check if this is solved already, as I don't want to 
>> > > keep an
>> > > unnecessary revert patch in our internal branches.
>> > > According to my check bug still exists.
>> > >
>> >
>> > I will handle this today, thanks for the reminder.
>>
>> Did you have a chance to do this? It looks like the original change
>> landed on mainline and causes modules to be autoloaded on KOBJ_UNBIND
>> again.
>>
>> Thanks!
>
> I sent the following to Tariq, and he tested it successfully.
>
> I will submit this formally.
>
> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> index 
> c3e84edc47c965d40199b652ba78876cdaa9c70c..0795482b15d5a8f1b65b570a071aa1419cb923d8
>  100644
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -346,19 +346,25 @@ static int kobject_uevent_net_broadcast(struct kobject 
> *kobj,
>  static void zap_modalias_env(struct kobj_uevent_env *env)
>  {
> static const char modalias_prefix[] = "MODALIAS=";
> +   size_t offset = 0, len;
> int i;
>
> for (i = 0; i < env->envp_idx;) {
> +   len = strlen(env->envp[i]) + 1;
> if (strncmp(env->envp[i], modalias_prefix,
> sizeof(modalias_prefix) - 1)) {
> i++;
> +   offset += len;
> continue;
> }
>
> -   if (i != env->envp_idx - 1)
> +   env->buflen -= len;
> +   if (i != env->envp_idx - 1) {
> +   memmove(env->envp[i], env->envp[i + 1],
> +   env->buflen - offset);
> memmove(>envp[i], >envp[i + 1],
> sizeof(env->envp[i]) * env->envp_idx - 1);
> -
> +   }
> env->envp_idx--;
> }
>  }
>

As I mentioned in the other thread, that works for netlink, but breaks
if you actually using env->envp pointers, as they also need to be
adjusted. I have a patch that fixes it properly.

Thanks!


-- 
Dmitry

Re: [PATCH v2 net-next 0/7] net: speedup netns create/delete time

[Dmitry Torokhov]

Hi Eric,

On Thu, Oct 19, 2017 at 7:11 AM, Eric Dumazet  wrote:
>
> On Thu, Oct 19, 2017 at 4:48 AM, Tariq Toukan  wrote:
> >
> > Hi Eric,
> >
> > I just wanted to check if this is solved already, as I don't want to keep an
> > unnecessary revert patch in our internal branches.
> > According to my check bug still exists.
> >
> I will handle this today, thanks for the reminder.

Did you have a chance to do this? It looks like the original change
landed on mainline and causes modules to be autoloaded on KOBJ_UNBIND
again.

Thanks!

-- 
Dmitry



-- 
Dmitry

Re: [PATCH v2 net-next 0/7] net: speedup netns create/delete time

[Dmitry Torokhov]

On September 26, 2017 8:13:21 AM PDT, Eric Dumazet <eduma...@google.com> wrote:
>On Tue, Sep 26, 2017 at 8:04 AM, Tariq Toukan <tar...@mellanox.com>
>wrote:
>>
>>
>> On 26/09/2017 3:51 PM, Eric Dumazet wrote:
>>>
>>> On Tue, Sep 26, 2017 at 4:21 AM, Tariq Toukan <tar...@mellanox.com>
>wrote:
>>>>
>>>>
>>>> Hi Eric,
>>>>
>>>> We see a regression introduced in this series, specifically in the
>>>> patches
>>>> touching lib/kobject_uevent.c.
>>>> We tried to figure out what is wrong there, but couldn't point it
>out.
>>>>
>>>> Bug is that mlx4 driver restart fails, because mlx4_core is still
>in use.
>>>> According to module dependencies, both mlx4_en and mlx4_ib should
>have
>>>> been
>>>> unloaded at this point
>>>> Please see log below.
>>>>
>>>> This looks to be some kind of a race, as the repro is not
>deterministic.
>>>> Probably the en/ib modules are now mistakenly reloaded.
>>>>
>>>> Any idea what could this be?
>>>>
>>>> Regards,
>>>> Tariq
>>>>
>>>>
>>>> [root@reg-l-vrt-41016-009 ~]# /etc/init.d/openibd stop
>>>> Unloading HCA driver:  [  OK  ]
>>>> [root@reg-l-vrt-41016-009 ~]# /etc/init.d/openibd start
>>>> Loading HCA driver and Access Layer:   [  OK  ]
>>>> [root@reg-l-vrt-41016-009 ~]# /etc/init.d/openibd stop
>>>> Unloading mlx4_core[FAILED]
>>>> rmmod: ERROR: Module mlx4_core is in use
>>>
>>> I have absolutely no idea. Please bisect.
>>
>> We previously saw a similar issue, that was reported in mailing list.
>> Dmitry Torokhov suggested the following fix:
>> https://lkml.org/lkml/2017/9/12/523
>>
>> And indeed, it solved the issue.
>>
>> We kept the suggested patch in our internal branch, and rebased.
>> Issue appeared again once your series was accepted.
>>
>> By bisecting, we see that the issue re-appears in this patch:
>> 4a336a23d619 kobject: copy env blob in one go
>>
>>>
>>> Are you really using netns in the first place ?
>>
>> No. But seems like it still affects the modules load/unload.
>>
>> Regards,
>> Tariq
>
>Ah this makes sense now.
>
>Dmitry Torokhov hack breaks the assumption I used in my patch.
>
>Since it is not upstream yet, I believe that it will need more work
>before being in a proper state.

It is in Greg's tree where all kobject patches should go through as far as I 
know.


Thanks.

-- 
Dmitry

Re: [PATCH 31/31] timer: Switch to testing for .function instead of .data

[Dmitry Torokhov]

On Thu, Aug 31, 2017 at 4:59 PM, Kees Cook <keesc...@chromium.org> wrote:
> On Thu, Aug 31, 2017 at 4:45 PM, Dmitry Torokhov
> <dmitry.torok...@gmail.com> wrote:
>> On Thu, Aug 31, 2017 at 4:29 PM, Kees Cook <keesc...@chromium.org> wrote:
>>> In several places, .data is checked for initialization to gate early
>>> calls to del_timer_sync(). Checking for .function is equally valid, so
>>> switch to this in all callers.
>>
>> Not seeing the rest of patches it is unclear from the patch
>> description why this is needed/wanted.
>
> The CC list would have been really giant, but here is the first patch
> and the earlier series list:
>
> https://lkml.org/lkml/2017/8/31/904
> https://lkml.org/lkml/2017/8/30/760
>
> tl;dr: We're going to switch all struct timer_list callbacks to get
> the timer pointer as the argument instead of from the .data field.
> This patch is one step in removing open-coded users of the .data
> field.
>

And that is exactly what should have been in the patch description.

FWIW for input bits:

Acked-by: Dmitry Torokhov <dmitry.torok...@gmail.com>

Thanks.

-- 
Dmitry

Re: [PATCH 31/31] timer: Switch to testing for .function instead of .data

[Dmitry Torokhov]

On Thu, Aug 31, 2017 at 4:29 PM, Kees Cook  wrote:
> In several places, .data is checked for initialization to gate early
> calls to del_timer_sync(). Checking for .function is equally valid, so
> switch to this in all callers.

Not seeing the rest of patches it is unclear from the patch
description why this is needed/wanted.

Thanks.

-- 
Dmitry

Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN

[Dmitry Torokhov]

On Fri, Jun 16, 2017 at 1:56 PM, Arnd Bergmann <a...@arndb.de> wrote:
> On Fri, Jun 16, 2017 at 7:29 PM, Dmitry Torokhov
> <dmitry.torok...@gmail.com> wrote:
>> On Fri, Jun 16, 2017 at 8:58 AM, Samuel Thibault
>> <samuel.thiba...@ens-lyon.org> wrote:
>>> I'm however afraid we'd have to mark a lot of static functions that way,
>>> depending on the aggressivity of gcc... I'd indeed really argue that gcc
>>> should consider stack usage when inlining.
>>>
>>> static int f(int foo) {
>>> char c[256];
>>> g(c, foo);
>>> }
>>>
>>> is really not something that I'd want to see the compiler to inline.
>>
>> Why would not we want it be inlined? What we do not want us several
>> calls having _separate_ instances of 'c' generated on the stack, all
>> inlined calls should share 'c'. And of course if we have f1, f2, and
>> f3 with c1, c2, and c3, GCC should not blow up the stack inlining and
>> allocating stack for all 3 of them beforehand.
>>
>> But this all seems to me issue that should be solved in toolchain, not
>> trying to play whack-a-mole with kernel sources.
>
> The problem for the Samuel's example is that
>
> a) the "--param asan-stack=1" option in KASAN does blow up the
>stack, which is why the annotation is now called 'noinline_if_stackbloat'.
>
> b) The toolchain cannot solve the problem, as most instances of the
>problem (unlike kbd_put_queue) force the inlining unless you build
>with the x86-specific CONFIG_OPTIMIZE_INLINING.

If inlining done right there should be no change in stack size,
because if calls are not inlined then stack storage is "shared"
between calls, and it should similarly be shared when calls are
inlined. And that is toolchain issue.

-- 
Dmitry

Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN

[Dmitry Torokhov]

On Fri, Jun 16, 2017 at 8:58 AM, Samuel Thibault
 wrote:
> Arnd Bergmann, on ven. 16 juin 2017 17:41:47 +0200, wrote:
>> The problem are the 'ch' and 'flag' variables that are passed into
>> tty_insert_flip_char by value, and from there into
>> tty_insert_flip_string_flags by reference.  In this case, kasan tries
>> to detect whether tty_insert_flip_string_flags() does any out-of-bounds
>> access on the pointers and adds 64 bytes redzone around each of
>> the two variables.
>
> Ouch.
>
>> gcc-6.3.1 happens to inline 16 calls of tty_insert_flip_char() into

I wonder if we should stop marking tty_insert_flip_char() as inline.

>> kbd_keycode(), so the stack size grows from 168 bytes to
>> 168+(16*2*64) = 2216 bytes. There are 10 calls to put_queue()
>> in to_utf8(), 12 in emulate_raw() and another 4 in kbd_keycode()
>> itself.
>
> That's why I agreed for put_queue :)
>
> I'm however afraid we'd have to mark a lot of static functions that way,
> depending on the aggressivity of gcc... I'd indeed really argue that gcc
> should consider stack usage when inlining.
>
> static int f(int foo) {
> char c[256];
> g(c, foo);
> }
>
> is really not something that I'd want to see the compiler to inline.

Why would not we want it be inlined? What we do not want us several
calls having _separate_ instances of 'c' generated on the stack, all
inlined calls should share 'c'. And of course if we have f1, f2, and
f3 with c1, c2, and c3, GCC should not blow up the stack inlining and
allocating stack for all 3 of them beforehand.

But this all seems to me issue that should be solved in toolchain, not
trying to play whack-a-mole with kernel sources.

Thanks.

-- 
Dmitry

[PATCH] drivers: net: ethernet: remove incorrect __exit markups

[Dmitry Torokhov]

Even if bus is not hot-pluggable, devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/net/ethernet/amd/declance.c| 30 +++---
 drivers/net/ethernet/broadcom/sb1250-mac.c |  4 ++--
 drivers/net/ethernet/faraday/ftgmac100.c   |  4 ++--
 drivers/net/ethernet/faraday/ftmac100.c|  4 ++--
 drivers/net/ethernet/seeq/sgiseeq.c|  4 ++--
 drivers/net/ethernet/sgi/meth.c|  4 ++--
 6 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/amd/declance.c 
b/drivers/net/ethernet/amd/declance.c
index 76e5fc7adff5..6c98901f1b89 100644
--- a/drivers/net/ethernet/amd/declance.c
+++ b/drivers/net/ethernet/amd/declance.c
@@ -1276,18 +1276,6 @@ static int dec_lance_probe(struct device *bdev, const 
int type)
return ret;
 }
 
-static void __exit dec_lance_remove(struct device *bdev)
-{
-   struct net_device *dev = dev_get_drvdata(bdev);
-   resource_size_t start, len;
-
-   unregister_netdev(dev);
-   start = to_tc_dev(bdev)->resource.start;
-   len = to_tc_dev(bdev)->resource.end - start + 1;
-   release_mem_region(start, len);
-   free_netdev(dev);
-}
-
 /* Find all the lance cards on the system and initialize them */
 static int __init dec_lance_platform_probe(void)
 {
@@ -1320,7 +1308,7 @@ static void __exit dec_lance_platform_remove(void)
 
 #ifdef CONFIG_TC
 static int dec_lance_tc_probe(struct device *dev);
-static int __exit dec_lance_tc_remove(struct device *dev);
+static int dec_lance_tc_remove(struct device *dev);
 
 static const struct tc_device_id dec_lance_tc_table[] = {
{ "DEC ", "PMAD-AA " },
@@ -1334,7 +1322,7 @@ static struct tc_driver dec_lance_tc_driver = {
.name   = "declance",
.bus= _bus_type,
.probe  = dec_lance_tc_probe,
-   .remove = __exit_p(dec_lance_tc_remove),
+   .remove = dec_lance_tc_remove,
},
 };
 
@@ -1346,7 +1334,19 @@ static int dec_lance_tc_probe(struct device *dev)
 return status;
 }
 
-static int __exit dec_lance_tc_remove(struct device *dev)
+static void dec_lance_remove(struct device *bdev)
+{
+   struct net_device *dev = dev_get_drvdata(bdev);
+   resource_size_t start, len;
+
+   unregister_netdev(dev);
+   start = to_tc_dev(bdev)->resource.start;
+   len = to_tc_dev(bdev)->resource.end - start + 1;
+   release_mem_region(start, len);
+   free_netdev(dev);
+}
+
+static int dec_lance_tc_remove(struct device *dev)
 {
 put_device(dev);
 dec_lance_remove(dev);
diff --git a/drivers/net/ethernet/broadcom/sb1250-mac.c 
b/drivers/net/ethernet/broadcom/sb1250-mac.c
index 435a2e4739d1..f82ec1e506e2 100644
--- a/drivers/net/ethernet/broadcom/sb1250-mac.c
+++ b/drivers/net/ethernet/broadcom/sb1250-mac.c
@@ -2617,7 +2617,7 @@ static int sbmac_probe(struct platform_device *pldev)
return err;
 }
 
-static int __exit sbmac_remove(struct platform_device *pldev)
+static int sbmac_remove(struct platform_device *pldev)
 {
struct net_device *dev = platform_get_drvdata(pldev);
struct sbmac_softc *sc = netdev_priv(dev);
@@ -2634,7 +2634,7 @@ static int __exit sbmac_remove(struct platform_device 
*pldev)
 
 static struct platform_driver sbmac_driver = {
.probe = sbmac_probe,
-   .remove = __exit_p(sbmac_remove),
+   .remove = sbmac_remove,
.driver = {
.name = sbmac_string,
},
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 262587240c86..928b0df2b8e0 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1456,7 +1456,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
return err;
 }
 
-static int __exit ftgmac100_remove(struct platform_device *pdev)
+static int ftgmac100_remove(struct platform_device *pdev)
 {
struct net_device *netdev;
struct ftgmac100 *priv;
@@ -1483,7 +1483,7 @@ MODULE_DEVICE_TABLE(of, ftgmac100_of_match);
 
 static struct platform_driver ftgmac100_driver = {
.probe  = ftgmac100_probe,
-   .remove = __exit_p(ftgmac100_remove),
+   .remove = ftgmac100_remove,
.driver = {
.name   = DRV_NAME,
.of_match_table = ftgmac100_of_match,
diff --git a/drivers/net/ethernet/faraday/ftmac100.c 
b/drivers/net/ethernet/faraday/ftmac100.c
index dce5f7b7f772..0f122a85a484 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -1154,7 +1154,7 @@ static int ftmac100_probe(struct platform_device *pdev)
return err

[PATCH v2 4/4] ptp: create "pins" together with the rest of attributes

[Dmitry Torokhov]

Let's switch to using device_create_with_groups(), which will allow us to
create "pins" attribute group together with the rest of ptp device
attributes, and before userspace gets notified about ptp device creation.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_clock.c   | 20 +++-
 drivers/ptp/ptp_private.h |  7 ---
 drivers/ptp/ptp_sysfs.c   | 39 +--
 3 files changed, 24 insertions(+), 42 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index b4e5e8022c29..e8142803a1a7 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -221,16 +221,17 @@ struct ptp_clock *ptp_clock_register(struct 
ptp_clock_info *info,
mutex_init(>pincfg_mux);
init_waitqueue_head(>tsev_wq);
 
+   err = ptp_populate_pin_groups(ptp);
+   if (err)
+   goto no_pin_groups;
+
/* Create a new device in our class. */
-   ptp->dev = device_create(ptp_class, parent, ptp->devid, ptp,
-"ptp%d", ptp->index);
+   ptp->dev = device_create_with_groups(ptp_class, parent, ptp->devid,
+ptp, ptp->pin_attr_groups,
+"ptp%d", ptp->index);
if (IS_ERR(ptp->dev))
goto no_device;
 
-   err = ptp_populate_sysfs(ptp);
-   if (err)
-   goto no_sysfs;
-
/* Register a new PPS source. */
if (info->pps) {
struct pps_source_info pps;
@@ -258,10 +259,10 @@ struct ptp_clock *ptp_clock_register(struct 
ptp_clock_info *info,
if (ptp->pps_source)
pps_unregister_source(ptp->pps_source);
 no_pps:
-   ptp_cleanup_sysfs(ptp);
-no_sysfs:
device_destroy(ptp_class, ptp->devid);
 no_device:
+   ptp_cleanup_pin_groups(ptp);
+no_pin_groups:
mutex_destroy(>tsevq_mux);
mutex_destroy(>pincfg_mux);
ida_simple_remove(_clocks_map, index);
@@ -280,8 +281,9 @@ int ptp_clock_unregister(struct ptp_clock *ptp)
/* Release the clock's resources. */
if (ptp->pps_source)
pps_unregister_source(ptp->pps_source);
-   ptp_cleanup_sysfs(ptp);
+
device_destroy(ptp_class, ptp->devid);
+   ptp_cleanup_pin_groups(ptp);
 
posix_clock_unregister(>clock);
return 0;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 9c5d41421b65..d95888974d0c 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -54,6 +54,8 @@ struct ptp_clock {
struct device_attribute *pin_dev_attr;
struct attribute **pin_attr;
struct attribute_group pin_attr_group;
+   /* 1st entry is a pointer to the real group, 2nd is NULL terminator */
+   const struct attribute_group *pin_attr_groups[2];
 };
 
 /*
@@ -94,8 +96,7 @@ uint ptp_poll(struct posix_clock *pc,
 
 extern const struct attribute_group *ptp_groups[];
 
-int ptp_cleanup_sysfs(struct ptp_clock *ptp);
-
-int ptp_populate_sysfs(struct ptp_clock *ptp);
+int ptp_populate_pin_groups(struct ptp_clock *ptp);
+void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
 
 #endif
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 426e42c51df4..48401dfcd999 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -268,25 +268,14 @@ static ssize_t ptp_pin_store(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
-int ptp_cleanup_sysfs(struct ptp_clock *ptp)
+int ptp_populate_pin_groups(struct ptp_clock *ptp)
 {
-   struct device *dev = ptp->dev;
-   struct ptp_clock_info *info = ptp->info;
-
-   if (info->n_pins) {
-   sysfs_remove_group(>kobj, >pin_attr_group);
-   kfree(ptp->pin_attr);
-   kfree(ptp->pin_dev_attr);
-   }
-   return 0;
-}
-
-static int ptp_populate_pins(struct ptp_clock *ptp)
-{
-   struct device *dev = ptp->dev;
struct ptp_clock_info *info = ptp->info;
int err = -ENOMEM, i, n_pins = info->n_pins;
 
+   if (!n_pins)
+   return 0;
+
ptp->pin_dev_attr = kcalloc(n_pins, sizeof(*ptp->pin_dev_attr),
GFP_KERNEL);
if (!ptp->pin_dev_attr)
@@ -309,28 +298,18 @@ static int ptp_populate_pins(struct ptp_clock *ptp)
ptp->pin_attr_group.name = "pins";
ptp->pin_attr_group.attrs = ptp->pin_attr;
 
-   err = sysfs_create_group(>kobj, >pin_attr_group);
-   if (err)
-   goto no_group;
+   ptp->pin_attr_groups[0] = >pin_attr_group;
+
return 0;
 
-no_group:
-   kfree(ptp->pin_attr);
 no_pin_attr:
kfree(ptp->pin_dev_attr);
 no_dev_attr:
return err;
 }
 
-int ptp_populate_sysfs(struct ptp_clock *ptp

[PATCH v2 3/4] ptp: use is_visible method to hide unused attributes

[Dmitry Torokhov]

Instead of creating selected attributes after the device is created (and
after userspace potentially seen uevent), lets use attribute group
is_visible() method to control which attributes are shown. This will allow
us to create all attributes (except "pins" group, which will be taken care
of later) before userspace gets notified about new ptp class device.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_sysfs.c | 125 +---
 1 file changed, 55 insertions(+), 70 deletions(-)

diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 27cd46ab5e32..426e42c51df4 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -46,27 +46,6 @@ PTP_SHOW_INT(n_periodic_outputs, n_per_out);
 PTP_SHOW_INT(n_programmable_pins, n_pins);
 PTP_SHOW_INT(pps_available, pps);
 
-static struct attribute *ptp_attrs[] = {
-   _attr_clock_name.attr,
-   _attr_max_adjustment.attr,
-   _attr_n_alarms.attr,
-   _attr_n_external_timestamps.attr,
-   _attr_n_periodic_outputs.attr,
-   _attr_n_programmable_pins.attr,
-   _attr_pps_available.attr,
-   NULL,
-};
-
-static const struct attribute_group ptp_group = {
-   .attrs = ptp_attrs,
-};
-
-const struct attribute_group *ptp_groups[] = {
-   _group,
-   NULL,
-};
-
-
 static ssize_t extts_enable_store(struct device *dev,
  struct device_attribute *attr,
  const char *buf, size_t count)
@@ -91,6 +70,7 @@ static ssize_t extts_enable_store(struct device *dev,
 out:
return err;
 }
+static DEVICE_ATTR(extts_enable, 0220, NULL, extts_enable_store);
 
 static ssize_t extts_fifo_show(struct device *dev,
   struct device_attribute *attr, char *page)
@@ -124,6 +104,7 @@ static ssize_t extts_fifo_show(struct device *dev,
mutex_unlock(>tsevq_mux);
return cnt;
 }
+static DEVICE_ATTR(fifo, 0444, extts_fifo_show, NULL);
 
 static ssize_t period_store(struct device *dev,
struct device_attribute *attr,
@@ -151,6 +132,7 @@ static ssize_t period_store(struct device *dev,
 out:
return err;
 }
+static DEVICE_ATTR(period, 0220, NULL, period_store);
 
 static ssize_t pps_enable_store(struct device *dev,
struct device_attribute *attr,
@@ -177,6 +159,57 @@ static ssize_t pps_enable_store(struct device *dev,
 out:
return err;
 }
+static DEVICE_ATTR(pps_enable, 0220, NULL, pps_enable_store);
+
+static struct attribute *ptp_attrs[] = {
+   _attr_clock_name.attr,
+
+   _attr_max_adjustment.attr,
+   _attr_n_alarms.attr,
+   _attr_n_external_timestamps.attr,
+   _attr_n_periodic_outputs.attr,
+   _attr_n_programmable_pins.attr,
+   _attr_pps_available.attr,
+
+   _attr_extts_enable.attr,
+   _attr_fifo.attr,
+   _attr_period.attr,
+   _attr_pps_enable.attr,
+   NULL
+};
+
+static umode_t ptp_is_attribute_visible(struct kobject *kobj,
+   struct attribute *attr, int n)
+{
+   struct device *dev = kobj_to_dev(kobj);
+   struct ptp_clock *ptp = dev_get_drvdata(dev);
+   struct ptp_clock_info *info = ptp->info;
+   umode_t mode = attr->mode;
+
+   if (attr == _attr_extts_enable.attr ||
+   attr == _attr_fifo.attr) {
+   if (!info->n_ext_ts)
+   mode = 0;
+   } else if (attr == _attr_period.attr) {
+   if (!info->n_per_out)
+   mode = 0;
+   } else if (attr == _attr_pps_enable.attr) {
+   if (!info->pps)
+   mode = 0;
+   }
+
+   return mode;
+}
+
+static const struct attribute_group ptp_group = {
+   .is_visible = ptp_is_attribute_visible,
+   .attrs  = ptp_attrs,
+};
+
+const struct attribute_group *ptp_groups[] = {
+   _group,
+   NULL
+};
 
 static int ptp_pin_name2index(struct ptp_clock *ptp, const char *name)
 {
@@ -235,26 +268,11 @@ static ssize_t ptp_pin_store(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
-static DEVICE_ATTR(extts_enable, 0220, NULL, extts_enable_store);
-static DEVICE_ATTR(fifo, 0444, extts_fifo_show, NULL);
-static DEVICE_ATTR(period,   0220, NULL, period_store);
-static DEVICE_ATTR(pps_enable,   0220, NULL, pps_enable_store);
-
 int ptp_cleanup_sysfs(struct ptp_clock *ptp)
 {
struct device *dev = ptp->dev;
struct ptp_clock_info *info = ptp->info;
 
-   if (info->n_ext_ts) {
-   device_remove_file(dev, _attr_extts_enable);
-   device_remove_file(dev, _attr_fifo);
-   }
-   if (info->n_per_out)
-   device_remove_file(dev, _attr_period);
-
-   if (info->pps)
-   device_remove_file(dev, _attr_pps_enable);
-
if (info->n_pins) {
 

[PATCH v2 2/4] ptp: use kcalloc when allocating arrays

[Dmitry Torokhov]

kcalloc is more semantically correct when allocating arrays of objects, and
overflow-safe.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_sysfs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 53d43954a974..27cd46ab5e32 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -269,13 +269,12 @@ static int ptp_populate_pins(struct ptp_clock *ptp)
struct ptp_clock_info *info = ptp->info;
int err = -ENOMEM, i, n_pins = info->n_pins;
 
-   ptp->pin_dev_attr = kzalloc(n_pins * sizeof(*ptp->pin_dev_attr),
+   ptp->pin_dev_attr = kcalloc(n_pins, sizeof(*ptp->pin_dev_attr),
GFP_KERNEL);
if (!ptp->pin_dev_attr)
goto no_dev_attr;
 
-   ptp->pin_attr = kzalloc((1 + n_pins) * sizeof(struct attribute *),
-   GFP_KERNEL);
+   ptp->pin_attr = kcalloc(1 + n_pins, sizeof(*ptp->pin_attr), GFP_KERNEL);
if (!ptp->pin_attr)
goto no_pin_attr;
 
-- 
2.11.0.483.g087da7b7c-goog

[PATCH v2 0/4] PTP attribute handling cleanup

[Dmitry Torokhov]

PTP core was creating some attributes, such as "period" and "fifo", and the
entire "pins" attribute group, after creating class deevice, which creates
a race for userspace: uevent may arrive before all attributes are created.

This series of patches switches PTP to use is_visible() to control
visibility of attributes in a group, and device_create_with_groups() to
ensure that attributes are created before we notify userspace of a new
device.

v2:
- added Richard's acked-by to patch #1
- removed use of kmalloc_array in favor of kcalloc in patch #2 at
  Richard's request
- added a cover letter

v1:
- initial patch set

Dmitry Torokhov (4):
  ptp: do not explicitly set drvdata in ptp_clock_register()
  ptp: use kcalloc when allocating arrays
  ptp: use is_visible method to hide unused attributes
  ptp: create "pins" together with the rest of attributes

 drivers/ptp/ptp_clock.c   |  22 +++---
 drivers/ptp/ptp_private.h |   7 +-
 drivers/ptp/ptp_sysfs.c   | 167 ++
 3 files changed, 80 insertions(+), 116 deletions(-)

Thanks.

-- 
Dmitry

[PATCH v2 1/4] ptp: do not explicitly set drvdata in ptp_clock_register()

[Dmitry Torokhov]

We do not need explicitly call dev_set_drvdata(), as it is done for us by
device_create().

Acked-by: Richard Cochran <richardcoch...@gmail.com>
Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_clock.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 9c13381b6966..b4e5e8022c29 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -227,8 +227,6 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info 
*info,
if (IS_ERR(ptp->dev))
goto no_device;
 
-   dev_set_drvdata(ptp->dev, ptp);
-
err = ptp_populate_sysfs(ptp);
if (err)
goto no_sysfs;
-- 
2.11.0.483.g087da7b7c-goog

Re: [PATCH 3/4] ptp: use is_visible method to hide unused attributes

[Dmitry Torokhov]

Hi Richard,

On Tue, Feb 14, 2017 at 09:42:26AM +0100, Richard Cochran wrote:
> On Mon, Feb 13, 2017 at 07:51:07PM -0800, Dmitry Torokhov wrote:
> > Instead of creating selected attributes after the device is created (and
> > after userspace potentially seen uevent), lets use attribute group
> > is_visible() method to control which attributes are shown. This will allow
> > us to create all attributes (except "pins" group, which will be taken care
> > of later) before userspace gets notified about new ptp class device.
> 
> At first glance, this patch and the next look like nice improvements.
> I don't futz around with sysfs code very often, and so may I ask how
> or whether you tested it?

I used the hack below.

Thanks.

-- 
Dmitry

PTP Test

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/net/ethernet/intel/e1000e/ptp.c |   13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c 
b/drivers/net/ethernet/intel/e1000e/ptp.c
index 34cc3be0df8e..3b4c0d3e4093 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -260,18 +260,27 @@ static void e1000e_systim_overflow_work(struct 
work_struct *work)
  E1000_SYSTIM_OVERFLOW_PERIOD);
 }
 
+static struct ptp_pin_desc e1000e_ptp_pin_config[1] = {
+   {
+   .name = "TestPin1",
+   .index = 0,
+   .func = PTP_PF_NONE,
+   }
+};
+
 static const struct ptp_clock_info e1000e_ptp_clock_info = {
.owner  = THIS_MODULE,
.n_alarm= 0,
.n_ext_ts   = 0,
-   .n_per_out  = 0,
-   .n_pins = 0,
+   .n_per_out  = 1,
+   .n_pins = 1,
.pps= 0,
.adjfreq= e1000e_phc_adjfreq,
.adjtime= e1000e_phc_adjtime,
.gettime64  = e1000e_phc_gettime,
.settime64  = e1000e_phc_settime,
.enable = e1000e_phc_enable,
+   .pin_config = e1000e_ptp_pin_config,
 };
 
 /**

[PATCH 2/4] ptp: use kcalloc/kmallco_array when allocating arrays

[Dmitry Torokhov]

kcalloc/kmalloc_array are more semantically correct when allocating arrays
of objects, and overflow-safe.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_sysfs.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 53d43954a974..77843790c381 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -269,13 +269,13 @@ static int ptp_populate_pins(struct ptp_clock *ptp)
struct ptp_clock_info *info = ptp->info;
int err = -ENOMEM, i, n_pins = info->n_pins;
 
-   ptp->pin_dev_attr = kzalloc(n_pins * sizeof(*ptp->pin_dev_attr),
+   ptp->pin_dev_attr = kcalloc(n_pins, sizeof(*ptp->pin_dev_attr),
GFP_KERNEL);
if (!ptp->pin_dev_attr)
goto no_dev_attr;
 
-   ptp->pin_attr = kzalloc((1 + n_pins) * sizeof(struct attribute *),
-   GFP_KERNEL);
+   ptp->pin_attr = kmalloc_array(1 + n_pins, sizeof(*ptp->pin_attr),
+ GFP_KERNEL);
if (!ptp->pin_attr)
goto no_pin_attr;
 
@@ -289,6 +289,9 @@ static int ptp_populate_pins(struct ptp_clock *ptp)
ptp->pin_attr[i] = >attr;
}
 
+   /* NULL terminator */
+   ptp->pin_attr[n_pins] = NULL;
+
ptp->pin_attr_group.name = "pins";
ptp->pin_attr_group.attrs = ptp->pin_attr;
 
-- 
2.11.0.483.g087da7b7c-goog

[PATCH 1/4] ptp: do not explicitly set drvdata in ptp_clock_register()

[Dmitry Torokhov]

We do not need explicitly call dev_set_drvdata(), as it is done for us by
device_create().

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_clock.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 9c13381b6966..b4e5e8022c29 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -227,8 +227,6 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info 
*info,
if (IS_ERR(ptp->dev))
goto no_device;
 
-   dev_set_drvdata(ptp->dev, ptp);
-
err = ptp_populate_sysfs(ptp);
if (err)
goto no_sysfs;
-- 
2.11.0.483.g087da7b7c-goog

[PATCH 4/4] ptp: create "pins" together with the rest of attributes

[Dmitry Torokhov]

Let's switch to using device_create_with_groups(), which will allow us to
create "pins" attribute group together with the rest of ptp device
attributes, and before userspace gets notified about ptp device creation.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_clock.c   | 20 +++-
 drivers/ptp/ptp_private.h |  7 ---
 drivers/ptp/ptp_sysfs.c   | 39 +--
 3 files changed, 24 insertions(+), 42 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index b4e5e8022c29..e8142803a1a7 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -221,16 +221,17 @@ struct ptp_clock *ptp_clock_register(struct 
ptp_clock_info *info,
mutex_init(>pincfg_mux);
init_waitqueue_head(>tsev_wq);
 
+   err = ptp_populate_pin_groups(ptp);
+   if (err)
+   goto no_pin_groups;
+
/* Create a new device in our class. */
-   ptp->dev = device_create(ptp_class, parent, ptp->devid, ptp,
-"ptp%d", ptp->index);
+   ptp->dev = device_create_with_groups(ptp_class, parent, ptp->devid,
+ptp, ptp->pin_attr_groups,
+"ptp%d", ptp->index);
if (IS_ERR(ptp->dev))
goto no_device;
 
-   err = ptp_populate_sysfs(ptp);
-   if (err)
-   goto no_sysfs;
-
/* Register a new PPS source. */
if (info->pps) {
struct pps_source_info pps;
@@ -258,10 +259,10 @@ struct ptp_clock *ptp_clock_register(struct 
ptp_clock_info *info,
if (ptp->pps_source)
pps_unregister_source(ptp->pps_source);
 no_pps:
-   ptp_cleanup_sysfs(ptp);
-no_sysfs:
device_destroy(ptp_class, ptp->devid);
 no_device:
+   ptp_cleanup_pin_groups(ptp);
+no_pin_groups:
mutex_destroy(>tsevq_mux);
mutex_destroy(>pincfg_mux);
ida_simple_remove(_clocks_map, index);
@@ -280,8 +281,9 @@ int ptp_clock_unregister(struct ptp_clock *ptp)
/* Release the clock's resources. */
if (ptp->pps_source)
pps_unregister_source(ptp->pps_source);
-   ptp_cleanup_sysfs(ptp);
+
device_destroy(ptp_class, ptp->devid);
+   ptp_cleanup_pin_groups(ptp);
 
posix_clock_unregister(>clock);
return 0;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 9c5d41421b65..d95888974d0c 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -54,6 +54,8 @@ struct ptp_clock {
struct device_attribute *pin_dev_attr;
struct attribute **pin_attr;
struct attribute_group pin_attr_group;
+   /* 1st entry is a pointer to the real group, 2nd is NULL terminator */
+   const struct attribute_group *pin_attr_groups[2];
 };
 
 /*
@@ -94,8 +96,7 @@ uint ptp_poll(struct posix_clock *pc,
 
 extern const struct attribute_group *ptp_groups[];
 
-int ptp_cleanup_sysfs(struct ptp_clock *ptp);
-
-int ptp_populate_sysfs(struct ptp_clock *ptp);
+int ptp_populate_pin_groups(struct ptp_clock *ptp);
+void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
 
 #endif
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index a6292744c7ed..f8813ca8801c 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -268,25 +268,14 @@ static ssize_t ptp_pin_store(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
-int ptp_cleanup_sysfs(struct ptp_clock *ptp)
+int ptp_populate_pin_groups(struct ptp_clock *ptp)
 {
-   struct device *dev = ptp->dev;
-   struct ptp_clock_info *info = ptp->info;
-
-   if (info->n_pins) {
-   sysfs_remove_group(>kobj, >pin_attr_group);
-   kfree(ptp->pin_attr);
-   kfree(ptp->pin_dev_attr);
-   }
-   return 0;
-}
-
-static int ptp_populate_pins(struct ptp_clock *ptp)
-{
-   struct device *dev = ptp->dev;
struct ptp_clock_info *info = ptp->info;
int err = -ENOMEM, i, n_pins = info->n_pins;
 
+   if (!n_pins)
+   return 0;
+
ptp->pin_dev_attr = kcalloc(n_pins, sizeof(*ptp->pin_dev_attr),
GFP_KERNEL);
if (!ptp->pin_dev_attr)
@@ -313,28 +302,18 @@ static int ptp_populate_pins(struct ptp_clock *ptp)
ptp->pin_attr_group.name = "pins";
ptp->pin_attr_group.attrs = ptp->pin_attr;
 
-   err = sysfs_create_group(>kobj, >pin_attr_group);
-   if (err)
-   goto no_group;
+   ptp->pin_attr_groups[0] = >pin_attr_group;
+
return 0;
 
-no_group:
-   kfree(ptp->pin_attr);
 no_pin_attr:
kfree(ptp->pin_dev_attr);
 no_dev_attr:
return err;
 }
 
-int ptp_populate_sysfs(struct ptp_clock *ptp

[PATCH 3/4] ptp: use is_visible method to hide unused attributes

[Dmitry Torokhov]

Instead of creating selected attributes after the device is created (and
after userspace potentially seen uevent), lets use attribute group
is_visible() method to control which attributes are shown. This will allow
us to create all attributes (except "pins" group, which will be taken care
of later) before userspace gets notified about new ptp class device.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/ptp/ptp_sysfs.c | 125 +---
 1 file changed, 55 insertions(+), 70 deletions(-)

diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 77843790c381..a6292744c7ed 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -46,27 +46,6 @@ PTP_SHOW_INT(n_periodic_outputs, n_per_out);
 PTP_SHOW_INT(n_programmable_pins, n_pins);
 PTP_SHOW_INT(pps_available, pps);
 
-static struct attribute *ptp_attrs[] = {
-   _attr_clock_name.attr,
-   _attr_max_adjustment.attr,
-   _attr_n_alarms.attr,
-   _attr_n_external_timestamps.attr,
-   _attr_n_periodic_outputs.attr,
-   _attr_n_programmable_pins.attr,
-   _attr_pps_available.attr,
-   NULL,
-};
-
-static const struct attribute_group ptp_group = {
-   .attrs = ptp_attrs,
-};
-
-const struct attribute_group *ptp_groups[] = {
-   _group,
-   NULL,
-};
-
-
 static ssize_t extts_enable_store(struct device *dev,
  struct device_attribute *attr,
  const char *buf, size_t count)
@@ -91,6 +70,7 @@ static ssize_t extts_enable_store(struct device *dev,
 out:
return err;
 }
+static DEVICE_ATTR(extts_enable, 0220, NULL, extts_enable_store);
 
 static ssize_t extts_fifo_show(struct device *dev,
   struct device_attribute *attr, char *page)
@@ -124,6 +104,7 @@ static ssize_t extts_fifo_show(struct device *dev,
mutex_unlock(>tsevq_mux);
return cnt;
 }
+static DEVICE_ATTR(fifo, 0444, extts_fifo_show, NULL);
 
 static ssize_t period_store(struct device *dev,
struct device_attribute *attr,
@@ -151,6 +132,7 @@ static ssize_t period_store(struct device *dev,
 out:
return err;
 }
+static DEVICE_ATTR(period, 0220, NULL, period_store);
 
 static ssize_t pps_enable_store(struct device *dev,
struct device_attribute *attr,
@@ -177,6 +159,57 @@ static ssize_t pps_enable_store(struct device *dev,
 out:
return err;
 }
+static DEVICE_ATTR(pps_enable, 0220, NULL, pps_enable_store);
+
+static struct attribute *ptp_attrs[] = {
+   _attr_clock_name.attr,
+
+   _attr_max_adjustment.attr,
+   _attr_n_alarms.attr,
+   _attr_n_external_timestamps.attr,
+   _attr_n_periodic_outputs.attr,
+   _attr_n_programmable_pins.attr,
+   _attr_pps_available.attr,
+
+   _attr_extts_enable.attr,
+   _attr_fifo.attr,
+   _attr_period.attr,
+   _attr_pps_enable.attr,
+   NULL
+};
+
+static umode_t ptp_is_attribute_visible(struct kobject *kobj,
+   struct attribute *attr, int n)
+{
+   struct device *dev = kobj_to_dev(kobj);
+   struct ptp_clock *ptp = dev_get_drvdata(dev);
+   struct ptp_clock_info *info = ptp->info;
+   umode_t mode = attr->mode;
+
+   if (attr == _attr_extts_enable.attr ||
+   attr == _attr_fifo.attr) {
+   if (!info->n_ext_ts)
+   mode = 0;
+   } else if (attr == _attr_period.attr) {
+   if (!info->n_per_out)
+   mode = 0;
+   } else if (attr == _attr_pps_enable.attr) {
+   if (!info->pps)
+   mode = 0;
+   }
+
+   return mode;
+}
+
+static const struct attribute_group ptp_group = {
+   .is_visible = ptp_is_attribute_visible,
+   .attrs  = ptp_attrs,
+};
+
+const struct attribute_group *ptp_groups[] = {
+   _group,
+   NULL
+};
 
 static int ptp_pin_name2index(struct ptp_clock *ptp, const char *name)
 {
@@ -235,26 +268,11 @@ static ssize_t ptp_pin_store(struct device *dev, struct 
device_attribute *attr,
return count;
 }
 
-static DEVICE_ATTR(extts_enable, 0220, NULL, extts_enable_store);
-static DEVICE_ATTR(fifo, 0444, extts_fifo_show, NULL);
-static DEVICE_ATTR(period,   0220, NULL, period_store);
-static DEVICE_ATTR(pps_enable,   0220, NULL, pps_enable_store);
-
 int ptp_cleanup_sysfs(struct ptp_clock *ptp)
 {
struct device *dev = ptp->dev;
struct ptp_clock_info *info = ptp->info;
 
-   if (info->n_ext_ts) {
-   device_remove_file(dev, _attr_extts_enable);
-   device_remove_file(dev, _attr_fifo);
-   }
-   if (info->n_per_out)
-   device_remove_file(dev, _attr_period);
-
-   if (info->pps)
-   device_remove_file(dev, _attr_pps_enable);
-
if (info->n_pins) {
 

Re: [PATCH v3 3/3] Bluetooth: hidp: fix possible might sleep error in hidp_session_thread

[Dmitry Torokhov]

Hi Jeffy,

On Sun, Feb 12, 2017 at 8:12 PM, Jeffy Chen  wrote:
> It looks like hidp_session_thread has same pattern as the issue reported in
> old rfcomm:
>
> while (1) {
> set_current_state(TASK_INTERRUPTIBLE);
> if (condition)
> break;
> // may call might_sleep here
> schedule();
> }
> __set_current_state(TASK_RUNNING);
>
> Which fixed at:
> dfb2fae Bluetooth: Fix nested sleeps
>
> So let's fix it at the same way, also follow the suggestion of:
> https://lwn.net/Articles/628628/
>
> Signed-off-by: Jeffy Chen 
> 1/ Fix could not wake up by wake attempts on original wait queues.
> 2/ Remove unnecessary memory barrier before wake_up_* functions.
>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  net/bluetooth/hidp/core.c | 33 ++---
>  1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 0bec458..076bc50 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -36,6 +36,7 @@
>  #define VERSION "1.2"
>
>  static DECLARE_RWSEM(hidp_session_sem);
> +static DECLARE_WAIT_QUEUE_HEAD(hidp_session_wq);
>  static LIST_HEAD(hidp_session_list);
>
>  static unsigned char hidp_keycode[256] = {
> @@ -1068,12 +1069,12 @@ static int hidp_session_start_sync(struct 
> hidp_session *session)
>   * Wake up session thread and notify it to stop. This is asynchronous and
>   * returns immediately. Call this whenever a runtime error occurs and you 
> want
>   * the session to stop.
> - * Note: wake_up_process() performs any necessary memory-barriers for us.
> + * Note: wake_up_interruptible() performs any necessary memory-barriers for 
> us.
>   */
>  static void hidp_session_terminate(struct hidp_session *session)
>  {
> atomic_inc(>terminate);
> -   wake_up_process(session->task);
> +   wake_up_interruptible(_session_wq);
>  }
>
>  /*
> @@ -1180,7 +1181,9 @@ static void hidp_session_run(struct hidp_session 
> *session)
> struct sock *ctrl_sk = session->ctrl_sock->sk;
> struct sock *intr_sk = session->intr_sock->sk;
> struct sk_buff *skb;
> +   DEFINE_WAIT_FUNC(wait, woken_wake_function);
>
> +   add_wait_queue(_session_wq, );
> for (;;) {
> /*
>  * This thread can be woken up two ways:
> @@ -1188,12 +1191,10 @@ static void hidp_session_run(struct hidp_session 
> *session)
>  *session->terminate flag and wakes this thread up.
>  *  - Via modifying the socket state of ctrl/intr_sock. This
>  *thread is woken up by ->sk_state_changed().
> -*
> -* Note: set_current_state() performs any necessary
> -* memory-barriers for us.
>  */
> -   set_current_state(TASK_INTERRUPTIBLE);
>
> +   /* Ensure session->terminate is updated */
> +   smp_mb__before_atomic();
> if (atomic_read(>terminate))
> break;
>
> @@ -1227,11 +1228,22 @@ static void hidp_session_run(struct hidp_session 
> *session)
> hidp_process_transmit(session, >ctrl_transmit,
>   session->ctrl_sock);
>
> -   schedule();
> +   wait_woken(, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
> }
> +   remove_wait_queue(_session_wq, );
>
> atomic_inc(>terminate);
> -   set_current_state(TASK_RUNNING);
> +
> +   /* Ensure session->terminate is updated */
> +   smp_mb__after_atomic();
> +}
> +
> +int hidp_session_wake_function(wait_queue_t *wait, unsigned int mode,
> +  int sync, void *key)

static?

> +{
> +   wake_up_interruptible(_session_wq);
> +
> +   return default_wake_function(wait, mode, sync, key);

I do not think you need to call default_wake_function() here: the
process is waiting on hidp_session_wq, so the wakeup above is what
will wake up the session. By the time you get to call
default_wake_function() task will already be in wrong state. I think
we should simply return "false" instead of calling
default_wake_function().

>  }
>
>  /*
> @@ -1244,7 +1256,8 @@ static void hidp_session_run(struct hidp_session 
> *session)
>  static int hidp_session_thread(void *arg)
>  {
> struct hidp_session *session = arg;
> -   wait_queue_t ctrl_wait, intr_wait;
> +   DEFINE_WAIT_FUNC(ctrl_wait, hidp_session_wake_function);
> +   DEFINE_WAIT_FUNC(intr_wait, hidp_session_wake_function);
>
> BT_DBG("session %p", session);
>
> @@ -1254,8 +1267,6 @@ static int hidp_session_thread(void *arg)
> set_user_nice(current, -15);
> hidp_set_timer(session);
>
> -   init_waitqueue_entry(_wait, current);
> -   init_waitqueue_entry(_wait, 

Re: [PATCH] net: Use ns_capable_noaudit() when determining net sysctl permissions

[Dmitry Torokhov]

On Sat, Oct 01, 2016 at 03:25:04AM -0400, David Miller wrote:
> From: Dmitry Torokhov <dmitry.torok...@gmail.com>
> Date: Fri, 30 Sep 2016 15:24:31 -0700
> 
> > From: Tyler Hicks <tyhi...@canonical.com>
> > 
> > The capability check should not be audited since it is only being used
> > to determine the inode permissions. A failed check does not indicate a
> > violation of security policy but, when an LSM is enabled, a denial audit
> > message was being generated.
> > 
> > The denial audit message caused confusion for some application authors
> > because root-running Go applications always triggered the denial. To
> > prevent this confusion, the capability check in net_ctl_permissions() is
> > switched to the noaudit variant.
> > 
> > BugLink: https://launchpad.net/bugs/1465724
> > 
> > Signed-off-by: Tyler Hicks <tyhi...@canonical.com>
> > Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>
> > Signed-off-by: James Morris <james.l.mor...@oracle.com>
> > [dtor: reapplied after e79c6a4fc923 ("net: make net namespace sysctls
> > belong to container's owner") accidentally reverted the change.]
> > Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
> 
> Applied, but please be clear in the future what tree a patch
> is targetting, in this case 'net-next'.
> 
> You can indicate this in the Subject line "[PATCH net-next]".

Sorry, will do next time.

Thanks.

-- 
Dmitry

Re: [PATCH v2 3/3] net: make net namespace sysctls belong to container's owner

[Dmitry Torokhov]

On Fri, Sep 30, 2016 at 01:21:27AM -0400, David Miller wrote:
> From: Dmitry Torokhov <dmitry.torok...@gmail.com>
> Date: Thu, 29 Sep 2016 08:46:05 -0700
> 
> > Hi David,
> > 
> > On Wed, Aug 10, 2016 at 2:36 PM, Dmitry Torokhov
> > <dmitry.torok...@gmail.com> wrote:
> >> If net namespace is attached to a user namespace let's make container's
> >> root owner of sysctls affecting said network namespace instead of global
> >> root.
> >>
> >> This also allows us to clean up net_ctl_permissions() because we do not
> >> need to fudge permissions anymore for the container's owner since it now
> >> owns the objects in question.
> >>
> >> Acked-by: "Eric W. Biederman" <ebied...@xmission.com>
> >> Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
> > 
> > I was looking at linux-next today, and I noticed that, when you merged
> > my patch, you basically reverted the following commit:
> > 
> > commit d6e0d306449bcb5fa3c80e7a3edf11d45abf9ae9
> > Author: Tyler Hicks <tyhi...@canonical.com>
> > Date:   Thu Jun 2 23:43:22 2016 -0500
> > 
> > net: Use ns_capable_noaudit() when determining net sysctl permissions
> 
> Please send me a fixup patch for this, sorry.

Just did, look for <20160930222431.GA30208@dtor-ws>

Thanks.

-- 
Dmitry

[PATCH] net: Use ns_capable_noaudit() when determining net sysctl permissions

[Dmitry Torokhov]

From: Tyler Hicks <tyhi...@canonical.com>

The capability check should not be audited since it is only being used
to determine the inode permissions. A failed check does not indicate a
violation of security policy but, when an LSM is enabled, a denial audit
message was being generated.

The denial audit message caused confusion for some application authors
because root-running Go applications always triggered the denial. To
prevent this confusion, the capability check in net_ctl_permissions() is
switched to the noaudit variant.

BugLink: https://launchpad.net/bugs/1465724

Signed-off-by: Tyler Hicks <tyhi...@canonical.com>
Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>
Signed-off-by: James Morris <james.l.mor...@oracle.com>
[dtor: reapplied after e79c6a4fc923 ("net: make net namespace sysctls
belong to container's owner") accidentally reverted the change.]
Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 net/sysctl_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index c5d37f4..9199813 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -44,7 +44,7 @@ static int net_ctl_permissions(struct ctl_table_header *head,
struct net *net = container_of(head->set, struct net, sysctls);
 
/* Allow network administrator to have same access as root. */
-   if (ns_capable(net->user_ns, CAP_NET_ADMIN)) {
+   if (ns_capable_noaudit(net->user_ns, CAP_NET_ADMIN)) {
int mode = (table->mode >> 6) & 7;
return (mode << 6) | (mode << 3) | mode;
}
-- 
2.8.0.rc3.226.g39d4020


-- 
Dmitry

Re: [PATCH v2 3/3] net: make net namespace sysctls belong to container's owner

[Dmitry Torokhov]

Hi David,

On Wed, Aug 10, 2016 at 2:36 PM, Dmitry Torokhov
<dmitry.torok...@gmail.com> wrote:
> If net namespace is attached to a user namespace let's make container's
> root owner of sysctls affecting said network namespace instead of global
> root.
>
> This also allows us to clean up net_ctl_permissions() because we do not
> need to fudge permissions anymore for the container's owner since it now
> owns the objects in question.
>
> Acked-by: "Eric W. Biederman" <ebied...@xmission.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>

I was looking at linux-next today, and I noticed that, when you merged
my patch, you basically reverted the following commit:

commit d6e0d306449bcb5fa3c80e7a3edf11d45abf9ae9
Author: Tyler Hicks <tyhi...@canonical.com>
Date:   Thu Jun 2 23:43:22 2016 -0500

net: Use ns_capable_noaudit() when determining net sysctl permissions

The capability check should not be audited since it is only being used
to determine the inode permissions. A failed check does not indicate a
violation of security policy but, when an LSM is enabled, a denial audit
message was being generated.

The denial audit message caused confusion for some application authors
because root-running Go applications always triggered the denial. To
prevent this confusion, the capability check in net_ctl_permissions() is
switched to the noaudit variant.

BugLink: https://launchpad.net/bugs/1465724

Signed-off-by: Tyler Hicks <tyhi...@canonical.com>
Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>
Signed-off-by: James Morris <james.l.mor...@oracle.com>

Thanks!

> ---
>  fs/proc/proc_sysctl.c  |  5 +
>  include/linux/sysctl.h |  4 
>  net/sysctl_net.c   | 29 -
>  3 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 5e57c3e..28f9085 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -430,6 +430,7 @@ static int sysctl_perm(struct ctl_table_header *head, 
> struct ctl_table *table, i
>  static struct inode *proc_sys_make_inode(struct super_block *sb,
> struct ctl_table_header *head, struct ctl_table *table)
>  {
> +   struct ctl_table_root *root = head->root;
> struct inode *inode;
> struct proc_inode *ei;
>
> @@ -457,6 +458,10 @@ static struct inode *proc_sys_make_inode(struct 
> super_block *sb,
> if (is_empty_dir(head))
> make_empty_dir_inode(inode);
> }
> +
> +   if (root->set_ownership)
> +   root->set_ownership(head, table, >i_uid, 
> >i_gid);
> +
>  out:
> return inode;
>  }
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index fa7bc29..55bec2f 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  /* For the /proc/sys support */
> @@ -156,6 +157,9 @@ struct ctl_table_root {
> struct ctl_table_set default_set;
> struct ctl_table_set *(*lookup)(struct ctl_table_root *root,
>struct nsproxy *namespaces);
> +   void (*set_ownership)(struct ctl_table_header *head,
> + struct ctl_table *table,
> + kuid_t *uid, kgid_t *gid);
> int (*permissions)(struct ctl_table_header *head, struct ctl_table 
> *table);
>  };
>
> diff --git a/net/sysctl_net.c b/net/sysctl_net.c
> index ed98c1f..5bc1a3d 100644
> --- a/net/sysctl_net.c
> +++ b/net/sysctl_net.c
> @@ -42,26 +42,37 @@ static int net_ctl_permissions(struct ctl_table_header 
> *head,
>struct ctl_table *table)
>  {
> struct net *net = container_of(head->set, struct net, sysctls);
> -   kuid_t root_uid = make_kuid(net->user_ns, 0);
> -   kgid_t root_gid = make_kgid(net->user_ns, 0);
>
> /* Allow network administrator to have same access as root. */
> -   if (ns_capable(net->user_ns, CAP_NET_ADMIN) ||
> -   uid_eq(root_uid, current_euid())) {
> +   if (ns_capable(net->user_ns, CAP_NET_ADMIN)) {
> int mode = (table->mode >> 6) & 7;
> return (mode << 6) | (mode << 3) | mode;
> }
> -   /* Allow netns root group to have the same access as the root group */
> -   if (in_egroup_p(root_gid)) {
> -   int mode = (table->mode >> 3) & 7;
> -   return (mode << 3) | mode;
> -   }
> +
> return tabl

Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container

[Dmitry Torokhov]

On Mon, Aug 29, 2016 at 5:38 AM, Eric W. Biederman
<ebied...@xmission.com> wrote:
> David Miller <da...@davemloft.net> writes:
>
>> From: Dmitry Torokhov <dmitry.torok...@gmail.com>
>> Date: Tue, 16 Aug 2016 15:33:10 -0700
>>
>>> There are objects in /sys hierarchy (/sys/class/net/) that logically belong
>>> to a namespace/container. Unfortunately all sysfs objects start their life
>>> belonging to global root, and while we could change ownership manually,
>>> keeping tracks of all objects that come and go is cumbersome. It would
>>> be better if kernel created them using correct uid/gid from the beginning.
>>>
>>> This series changes kernfs to allow creating object's with arbitrary
>>> uid/gid, adds get_ownership() callback to ktype structure so subsystems
>>> could supply their own logic (likely tied to namespace support) for
>>> determining ownership of kobjects, and adjusts sysfs code to make use of
>>> this information. Lastly net-sysfs is adjusted to make sure that objects in
>>> net namespace are owned by the root user from the owning user namespace.
>>>
>>> Note that we do not adjust ownership of objects moved into a new namespace
>>> (as when moving a network device into a container) as userspace can easily
>>> do it.
>>
>> I need some domain experts to review this series please.
>
> I just came back from vacation and I will aim to take a look shortly.
>
> The big picture idea seems sensible.  Having a better ownship of sysfs
> files that are part of a network namespace.  I will have to look at the
> details to see if the implementation is similarly sensible.

Eric,

Did you find anything objectionable in the series or should I fix up
the !CONFIG_SYSFS error in networking patch and resubmit?

Thanks.

-- 
Dmitry

Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container

[Dmitry Torokhov]

On Sun, Aug 21, 2016 at 11:41:39PM -0700, David Miller wrote:
> From: Dmitry Torokhov <dmitry.torok...@gmail.com>
> Date: Tue, 16 Aug 2016 15:33:10 -0700
> 
> > There are objects in /sys hierarchy (/sys/class/net/) that logically belong
> > to a namespace/container. Unfortunately all sysfs objects start their life
> > belonging to global root, and while we could change ownership manually,
> > keeping tracks of all objects that come and go is cumbersome. It would
> > be better if kernel created them using correct uid/gid from the beginning.
> > 
> > This series changes kernfs to allow creating object's with arbitrary
> > uid/gid, adds get_ownership() callback to ktype structure so subsystems
> > could supply their own logic (likely tied to namespace support) for
> > determining ownership of kobjects, and adjusts sysfs code to make use of
> > this information. Lastly net-sysfs is adjusted to make sure that objects in
> > net namespace are owned by the root user from the owning user namespace.
> > 
> > Note that we do not adjust ownership of objects moved into a new namespace
> > (as when moving a network device into a container) as userspace can easily
> > do it.
> 
> As shown by the kbuild robot, this fails to build when CONFIG_SYSFS
> it disabled.

Sorry about that, I haven't noticed that all netdev queue stuff is
protected by #ifdef CONFIG_SYSFS. I'll move the definition out and
resubmit in a few days (maybe Eric and Tejun will have some comments for
be by then as well...).

Thanks.

-- 
Dmitry

[PATCH 4/5] driver core: set up ownership of class devices in sysfs

[Dmitry Torokhov]

Plumb in get_ownership() callback for devices belonging to a class so that
they can be created with uid/gid different from global root. This will
allow network devices in a container to belong to container's root and not
global root.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 drivers/base/core.c| 9 +
 include/linux/device.h | 5 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 0a8bdad..0f95454 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -263,10 +263,19 @@ static const void *device_namespace(struct kobject *kobj)
return ns;
 }
 
+static void device_get_ownership(struct kobject *kobj, kuid_t *uid, kgid_t 
*gid)
+{
+   struct device *dev = kobj_to_dev(kobj);
+
+   if (dev->class && dev->class->get_ownership)
+   dev->class->get_ownership(dev, uid, gid);
+}
+
 static struct kobj_type device_ktype = {
.release= device_release,
.sysfs_ops  = _sysfs_ops,
.namespace  = device_namespace,
+   .get_ownership  = device_get_ownership,
 };
 
 
diff --git a/include/linux/device.h b/include/linux/device.h
index 38f0281..da20b12 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -374,6 +374,9 @@ int subsys_virtual_register(struct bus_type *subsys,
  * @resume:Used to bring the device from the sleep mode.
  * @ns_type:   Callbacks so sysfs can detemine namespaces.
  * @namespace: Namespace of the device belongs to this class.
+ * @get_ownership: Allows class to specify uid/gid of the sysfs directories
+ * for the devices belonging to the class. Usually tied to
+ * device's namespace.
  * @pm:The default device power management operations of this 
class.
  * @p: The private data of the driver core, no one other than the
  * driver core can touch this.
@@ -404,6 +407,8 @@ struct class {
const struct kobj_ns_type_operations *ns_type;
const void *(*namespace)(struct device *dev);
 
+   void (*get_ownership)(struct device *dev, kuid_t *uid, kgid_t *gid);
+
const struct dev_pm_ops *pm;
 
struct subsys_private *p;
-- 
2.8.0.rc3.226.g39d4020

[PATCH 0/5] Make /sys/class/net per net namespace objects belong to container

[Dmitry Torokhov]

There are objects in /sys hierarchy (/sys/class/net/) that logically belong
to a namespace/container. Unfortunately all sysfs objects start their life
belonging to global root, and while we could change ownership manually,
keeping tracks of all objects that come and go is cumbersome. It would
be better if kernel created them using correct uid/gid from the beginning.

This series changes kernfs to allow creating object's with arbitrary
uid/gid, adds get_ownership() callback to ktype structure so subsystems
could supply their own logic (likely tied to namespace support) for
determining ownership of kobjects, and adjusts sysfs code to make use of
this information. Lastly net-sysfs is adjusted to make sure that objects in
net namespace are owned by the root user from the owning user namespace.

Note that we do not adjust ownership of objects moved into a new namespace
(as when moving a network device into a container) as userspace can easily
do it.

Thanks!

Dmitry Torokhov (5):
  kernfs: allow creating kernfs objects with arbitrary uid/gid
  sysfs, kobject: allow creating kobject belonging to arbitrary users
  kobject: kset_create_and_add() - fetch ownership info from parent
  driver core: set up ownership of class devices in sysfs
  net-sysfs: make sure objects belong to contrainer's owner

 drivers/base/core.c |  9 +
 fs/kernfs/dir.c | 29 ++---
 fs/kernfs/file.c|  8 ++--
 fs/kernfs/inode.c   |  2 +-
 fs/kernfs/kernfs-internal.h |  2 ++
 fs/kernfs/symlink.c | 11 ++-
 fs/sysfs/dir.c  |  7 ++-
 fs/sysfs/file.c | 33 +
 fs/sysfs/group.c| 23 +--
 fs/sysfs/sysfs.h|  3 ++-
 include/linux/device.h  |  5 +
 include/linux/kernfs.h  | 28 +++-
 include/linux/kobject.h |  4 
 kernel/cgroup.c |  4 +++-
 lib/kobject.c   | 28 +++-
 net/core/net-sysfs.c| 44 +++-
 16 files changed, 201 insertions(+), 39 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[PATCH 1/5] kernfs: allow creating kernfs objects with arbitrary uid/gid

[Dmitry Torokhov]

This change allows creating kernfs files and directories with arbitrary
uid/gid instead of always using GLOBAL_ROOT_UID/GID by extending
kernfs_create_dir_ns() and kernfs_create_file_ns() with uid/gid arguments.
The "simple" kernfs_create_file() and kernfs_create_dir() are left alone
and always create objects belonging to the global root.

When creating symlinks ownership (uid/gid) is taken from the target kernfs
object.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 fs/kernfs/dir.c | 29 ++---
 fs/kernfs/file.c|  8 ++--
 fs/kernfs/inode.c   |  2 +-
 fs/kernfs/kernfs-internal.h |  2 ++
 fs/kernfs/symlink.c | 11 ++-
 fs/sysfs/dir.c  |  4 +++-
 fs/sysfs/file.c |  5 +++--
 include/linux/kernfs.h  | 28 +++-
 kernel/cgroup.c |  4 +++-
 9 files changed, 73 insertions(+), 20 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 8a65240..67c276a 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -657,6 +657,7 @@ struct kernfs_node *kernfs_node_from_dentry(struct dentry 
*dentry)
 
 static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
 const char *name, umode_t mode,
+kuid_t uid, kgid_t gid,
 unsigned flags)
 {
struct kernfs_node *kn;
@@ -683,8 +684,22 @@ static struct kernfs_node *__kernfs_new_node(struct 
kernfs_root *root,
kn->mode = mode;
kn->flags = flags;
 
+   if (!uid_eq(uid, GLOBAL_ROOT_UID) || !gid_eq(gid, GLOBAL_ROOT_GID)) {
+   struct iattr iattr = {
+   .ia_valid = ATTR_UID | ATTR_GID,
+   .ia_uid = uid,
+   .ia_gid = gid,
+   };
+
+   ret = __kernfs_setattr(kn, );
+   if (ret < 0)
+   goto err_out3;
+   }
+
return kn;
 
+ err_out3:
+   ida_simple_remove(>ino_ida, kn->ino);
  err_out2:
kmem_cache_free(kernfs_node_cache, kn);
  err_out1:
@@ -694,11 +709,13 @@ static struct kernfs_node *__kernfs_new_node(struct 
kernfs_root *root,
 
 struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
const char *name, umode_t mode,
+   kuid_t uid, kgid_t gid,
unsigned flags)
 {
struct kernfs_node *kn;
 
-   kn = __kernfs_new_node(kernfs_root(parent), name, mode, flags);
+   kn = __kernfs_new_node(kernfs_root(parent),
+  name, mode, uid, gid, flags);
if (kn) {
kernfs_get(parent);
kn->parent = parent;
@@ -919,6 +936,7 @@ struct kernfs_root *kernfs_create_root(struct 
kernfs_syscall_ops *scops,
INIT_LIST_HEAD(>supers);
 
kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
+  GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
   KERNFS_DIR);
if (!kn) {
ida_destroy(>ino_ida);
@@ -957,6 +975,8 @@ void kernfs_destroy_root(struct kernfs_root *root)
  * @parent: parent in which to create a new directory
  * @name: name of the new directory
  * @mode: mode of the new directory
+ * @uid: uid of the new directory
+ * @gid: gid of the new directory
  * @priv: opaque data associated with the new directory
  * @ns: optional namespace tag of the directory
  *
@@ -964,13 +984,15 @@ void kernfs_destroy_root(struct kernfs_root *root)
  */
 struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
 const char *name, umode_t mode,
+kuid_t uid, kgid_t gid,
 void *priv, const void *ns)
 {
struct kernfs_node *kn;
int rc;
 
/* allocate */
-   kn = kernfs_new_node(parent, name, mode | S_IFDIR, KERNFS_DIR);
+   kn = kernfs_new_node(parent, name, mode | S_IFDIR,
+uid, gid, KERNFS_DIR);
if (!kn)
return ERR_PTR(-ENOMEM);
 
@@ -1001,7 +1023,8 @@ struct kernfs_node *kernfs_create_empty_dir(struct 
kernfs_node *parent,
int rc;
 
/* allocate */
-   kn = kernfs_new_node(parent, name, S_IRUGO|S_IXUGO|S_IFDIR, KERNFS_DIR);
+   kn = kernfs_new_node(parent, name, S_IRUGO|S_IXUGO|S_IFDIR,
+GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, KERNFS_DIR);
if (!kn)
return ERR_PTR(-ENOMEM);
 
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index e157400..736a168 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -904,6 +904,8 @@ const struct file_operations kernfs_file_fops = {
  * @parent: directory to create the file in
  * @name: name of the file
  * @mode: m

[PATCH 5/5] net-sysfs: make sure objects belong to contrainer's owner

[Dmitry Torokhov]

When creating various objects in /sys/class/net/... make sure that they
belong to container's owner instead of global root (if they belong to a
container/namespace).

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 net/core/net-sysfs.c | 44 +++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 7a0b616..ef997bb 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -882,11 +882,35 @@ static const void *rx_queue_namespace(struct kobject 
*kobj)
return ns;
 }
 
+static void net_ns_get_ownership(const struct net *net,
+kuid_t *uid, kgid_t *gid)
+{
+   if (net) {
+   kuid_t ns_root_uid = make_kuid(net->user_ns, 0);
+   kgid_t ns_root_gid = make_kgid(net->user_ns, 0);
+
+   if (uid_valid(ns_root_uid))
+   *uid = ns_root_uid;
+
+   if (gid_valid(ns_root_gid))
+   *gid = ns_root_gid;
+   }
+}
+
+static void rx_queue_get_ownership(struct kobject *kobj,
+  kuid_t *uid, kgid_t *gid)
+{
+   const struct net *net = rx_queue_namespace(kobj);
+
+   net_ns_get_ownership(net, uid, gid);
+}
+
 static struct kobj_type rx_queue_ktype = {
.sysfs_ops = _queue_sysfs_ops,
.release = rx_queue_release,
.default_attrs = rx_queue_default_attrs,
-   .namespace = rx_queue_namespace
+   .namespace = rx_queue_namespace,
+   .get_ownership = rx_queue_get_ownership,
 };
 
 static int rx_queue_add_kobject(struct net_device *dev, int index)
@@ -1274,11 +1298,20 @@ static const void *netdev_queue_namespace(struct 
kobject *kobj)
return ns;
 }
 
+static void netdev_queue_get_ownership(struct kobject *kobj,
+  kuid_t *uid, kgid_t *gid)
+{
+   const struct net *net = netdev_queue_namespace(kobj);
+
+   net_ns_get_ownership(net, uid, gid);
+}
+
 static struct kobj_type netdev_queue_ktype = {
.sysfs_ops = _queue_sysfs_ops,
.release = netdev_queue_release,
.default_attrs = netdev_queue_default_attrs,
.namespace = netdev_queue_namespace,
+   .get_ownership = netdev_queue_get_ownership,
 };
 
 static int netdev_queue_add_kobject(struct net_device *dev, int index)
@@ -1463,6 +1496,14 @@ static const void *net_namespace(struct device *d)
return dev_net(dev);
 }
 
+static void net_get_ownership(struct device *d, kuid_t *uid, kgid_t *gid)
+{
+   struct net_device *dev = to_net_dev(d);
+   const struct net *net = dev_net(dev);
+
+   net_ns_get_ownership(net, uid, gid);
+}
+
 static struct class net_class = {
.name = "net",
.dev_release = netdev_release,
@@ -1470,6 +1511,7 @@ static struct class net_class = {
.dev_uevent = netdev_uevent,
.ns_type = _ns_type_operations,
.namespace = net_namespace,
+   .get_ownership = net_get_ownership,
 };
 
 #ifdef CONFIG_OF_NET
-- 
2.8.0.rc3.226.g39d4020

[PATCH 3/5] kobject: kset_create_and_add() - fetch ownership info from parent

[Dmitry Torokhov]

This change implements get_ownership() for ksets created with
kset_create_and_add() call by fetching ownership data from parent kobject.
This is done mostly for benefit of "queues" attribute of net devices so
that corresponding directory belongs to container's root instead of global
root for network devices in a container.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 lib/kobject.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index 7d516d9..f81ac7c 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -890,9 +890,16 @@ static void kset_release(struct kobject *kobj)
kfree(kset);
 }
 
+void kset_get_ownership(struct kobject *kobj, kuid_t *uid, kgid_t *gid)
+{
+   if (kobj->parent)
+   kobject_get_ownership(kobj->parent, uid, gid);
+}
+
 static struct kobj_type kset_ktype = {
.sysfs_ops  = _sysfs_ops,
-   .release = kset_release,
+   .release= kset_release,
+   .get_ownership  = kset_get_ownership,
 };
 
 /**
-- 
2.8.0.rc3.226.g39d4020

[PATCH 2/5] sysfs, kobject: allow creating kobject belonging to arbitrary users

[Dmitry Torokhov]

Normally kobjects and their sysfs representation belong to global root,
however it is not necessarily the case for objects in separate namespaces.
For example, objects in separate network namespace logically belong to the
container's root and not global root.

This change lays groundwork for allowing network namespace objects
ownership to be transferred to container's root user by defining
get_ownership() callback in ktype structure and using it in sysfs code to
retrieve desired uid/gid when creating sysfs objects for given kobject.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 fs/sysfs/dir.c  |  7 +--
 fs/sysfs/file.c | 32 
 fs/sysfs/group.c| 23 +--
 fs/sysfs/sysfs.h|  3 ++-
 include/linux/kobject.h |  4 
 lib/kobject.c   | 19 +++
 6 files changed, 67 insertions(+), 21 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index d6a2b76..7ef6524 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -41,6 +41,8 @@ void sysfs_warn_dup(struct kernfs_node *parent, const char 
*name)
 int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
 {
struct kernfs_node *parent, *kn;
+   kuid_t uid;
+   kgid_t gid;
 
BUG_ON(!kobj);
 
@@ -52,9 +54,10 @@ int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
if (!parent)
return -ENOENT;
 
+   kobject_get_ownership(kobj, , );
+
kn = kernfs_create_dir_ns(parent, kobject_name(kobj),
- S_IRWXU | S_IRUGO | S_IXUGO,
- GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+ S_IRWXU | S_IRUGO | S_IXUGO, uid, gid,
  kobj, ns);
if (IS_ERR(kn)) {
if (PTR_ERR(kn) == -EEXIST)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 0241830..8411e7c 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -239,7 +239,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
   const struct attribute *attr, bool is_bin,
-  umode_t mode, const void *ns)
+  umode_t mode, kuid_t uid, kgid_t gid, const void *ns)
 {
struct lock_class_key *key = NULL;
const struct kernfs_ops *ops;
@@ -296,8 +296,8 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent,
if (!attr->ignore_lockdep)
key = attr->key ?: (struct lock_class_key *)>skey;
 #endif
-   kn = __kernfs_create_file(parent, attr->name,
- mode & 0777, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+
+   kn = __kernfs_create_file(parent, attr->name, mode & 0777, uid, gid,
  size, ops, (void *)attr, ns, key);
if (IS_ERR(kn)) {
if (PTR_ERR(kn) == -EEXIST)
@@ -307,12 +307,6 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent,
return 0;
 }
 
-int sysfs_add_file(struct kernfs_node *parent, const struct attribute *attr,
-  bool is_bin)
-{
-   return sysfs_add_file_mode_ns(parent, attr, is_bin, attr->mode, NULL);
-}
-
 /**
  * sysfs_create_file_ns - create an attribute file for an object with custom ns
  * @kobj: object we're creating for
@@ -322,9 +316,14 @@ int sysfs_add_file(struct kernfs_node *parent, const 
struct attribute *attr,
 int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr,
 const void *ns)
 {
+   kuid_t uid;
+   kgid_t gid;
+
BUG_ON(!kobj || !kobj->sd || !attr);
 
-   return sysfs_add_file_mode_ns(kobj->sd, attr, false, attr->mode, ns);
+   kobject_get_ownership(kobj, , );
+   return sysfs_add_file_mode_ns(kobj->sd, attr, false, attr->mode,
+ uid, gid, ns);
 
 }
 EXPORT_SYMBOL_GPL(sysfs_create_file_ns);
@@ -353,6 +352,8 @@ int sysfs_add_file_to_group(struct kobject *kobj,
const struct attribute *attr, const char *group)
 {
struct kernfs_node *parent;
+   kuid_t uid;
+   kgid_t gid;
int error;
 
if (group) {
@@ -365,7 +366,9 @@ int sysfs_add_file_to_group(struct kobject *kobj,
if (!parent)
return -ENOENT;
 
-   error = sysfs_add_file(parent, attr, false);
+   kobject_get_ownership(kobj, , );
+   error = sysfs_add_file_mode_ns(kobj->sd, attr, false,
+  attr->mode, uid, gid, NULL);
kernfs_put(parent);
 
return error;
@@ -481,9 +484,14 @@ EXPORT_SYMBOL_GPL(sysfs_remove_file_from_group);
 int sysfs_create_bin_file(struct kobject *kobj,
  const struct bin_attribute *attr)
 {
+   kuid_t uid;
+   kgid_t gid;
+
BUG_ON(!kobj || !kobj->sd || !attr);
 
-   return sysfs_add_file(ko

[PATCH v2 2/3] proc: make proc entries inherit ownership from parent

[Dmitry Torokhov]

There are certain parameters that belong to net namespace and that are
exported in /proc. They should be controllable by the container's owner,
but are currently owned by global root and thus not available.

Let's change proc code to inherit ownership of parent entry, and when
create per-ns "net" proc entry set it up as owned by container's owner.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 fs/proc/generic.c  |  2 ++
 fs/proc/proc_net.c | 13 +
 2 files changed, 15 insertions(+)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index c633476..bca66d8 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -390,6 +390,8 @@ static struct proc_dir_entry *__proc_create(struct 
proc_dir_entry **parent,
atomic_set(>count, 1);
spin_lock_init(>pde_unload_lock);
INIT_LIST_HEAD(>pde_openers);
+   proc_set_user(ent, (*parent)->uid, (*parent)->gid);
+
 out:
return ent;
 }
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index c8bbc68..7ae6b1d 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -185,6 +186,8 @@ const struct file_operations proc_net_operations = {
 static __net_init int proc_net_ns_init(struct net *net)
 {
struct proc_dir_entry *netd, *net_statd;
+   kuid_t uid;
+   kgid_t gid;
int err;
 
err = -ENOMEM;
@@ -199,6 +202,16 @@ static __net_init int proc_net_ns_init(struct net *net)
netd->parent = _root;
memcpy(netd->name, "net", 4);
 
+   uid = make_kuid(net->user_ns, 0);
+   if (!uid_valid(uid))
+   uid = netd->uid;
+
+   gid = make_kgid(net->user_ns, 0);
+   if (!gid_valid(gid))
+   gid = netd->gid;
+
+   proc_set_user(netd, uid, gid);
+
err = -EEXIST;
net_statd = proc_net_mkdir(net, "stat", netd);
if (!net_statd)
-- 
2.8.0.rc3.226.g39d4020

[PATCH v2 0/3] Make /proc per net namespace objects belong to container

[Dmitry Torokhov]

Currently [almost] all /proc objects belong to the global root, even if
data belongs to a given namespace within a container and (at least for
sysctls) we work around permssions checks to allow container's root to
access the data.

This series changes ownership of net namespace /proc objects
(/proc/net/self/* and /proc/sys/net/*) to be container's root and not
global root when there exists mapping for container's root in user
namespace.

This helps when running Android CTS in a container, but I think it makes
sense regardless.

Changes from V1:

- added fix for crash when !CONFIG_NET_NS (new patch #1)
- addressed Eric'c comments for error handling style in patch #3 and
  added his Ack
- adjusted patch #2 to use the same style of erro handling
- sent out as series instead of separate patches

Dmitry Torokhov (3):
  netns: do not call pernet ops for not yet set up init_net namespace
  proc: make proc entries inherit ownership from parent
  net: make net namespace sysctls belong to container's owner

 fs/proc/generic.c|  2 ++
 fs/proc/proc_net.c   | 13 +
 fs/proc/proc_sysctl.c|  5 +
 include/linux/sysctl.h   |  4 
 net/core/net_namespace.c | 21 +
 net/sysctl_net.c | 29 -
 6 files changed, 61 insertions(+), 13 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[PATCH v2 1/3] netns: do not call pernet ops for not yet set up init_net namespace

[Dmitry Torokhov]

When CONFIG_NET_NS is disabled, registering pernet operations causes
init() to be called immediately with init_net as an argument. Unfortunately
this leads to some pernet ops, such as proc_net_ns_init() to be called too
early, when init_net namespace has not been fully initialized. This causes
issues when we want to change pernet ops to use more data from the net
namespace in question, for example reference user namespace that owns our
network namespace.

To fix this we could either play game of musical chairs and rearrange init
order, or we could do the same as when CONFIG_NET_NS is enabled, and
postpone calling pernet ops->init() until namespace is set up properly.

Note that we can not simply undo commit ed160e839d2e ("[NET]: Cleanup
pernet operation without CONFIG_NET_NS") and use the same implementations
for __register_pernet_operations() and __unregister_pernet_operations(),
because many pernet ops are marked as __net_initdata and will be discarded,
which wreaks havoc on our ops lists. Here we rely on the fact that we only
use lists until init_net is fully initialized, which happens much earlier
than discarding __net_initdata sections.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 net/core/net_namespace.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 2c2eb1b..1fe5816 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -37,6 +37,8 @@ struct net init_net = {
 };
 EXPORT_SYMBOL(init_net);
 
+static bool init_net_initialized;
+
 #define INITIAL_NET_GEN_PTRS   13 /* +1 for len +2 for rcu_head */
 
 static unsigned int max_gen_ptrs = INITIAL_NET_GEN_PTRS;
@@ -750,6 +752,8 @@ static int __init net_ns_init(void)
if (setup_net(_net, _user_ns))
panic("Could not setup the initial network namespace");
 
+   init_net_initialized = true;
+
rtnl_lock();
list_add_tail_rcu(_net.list, _namespace_list);
rtnl_unlock();
@@ -811,15 +815,24 @@ static void __unregister_pernet_operations(struct 
pernet_operations *ops)
 static int __register_pernet_operations(struct list_head *list,
struct pernet_operations *ops)
 {
+   if (!init_net_initialized) {
+   list_add_tail(>list, list);
+   return 0;
+   }
+
return ops_init(ops, _net);
 }
 
 static void __unregister_pernet_operations(struct pernet_operations *ops)
 {
-   LIST_HEAD(net_exit_list);
-   list_add(_net.exit_list, _exit_list);
-   ops_exit_list(ops, _exit_list);
-   ops_free_list(ops, _exit_list);
+   if (!init_net_initialized) {
+   list_del(>list);
+   } else {
+   LIST_HEAD(net_exit_list);
+   list_add(_net.exit_list, _exit_list);
+   ops_exit_list(ops, _exit_list);
+   ops_free_list(ops, _exit_list);
+   }
 }
 
 #endif /* CONFIG_NET_NS */
-- 
2.8.0.rc3.226.g39d4020

[PATCH v2 3/3] net: make net namespace sysctls belong to container's owner

[Dmitry Torokhov]

If net namespace is attached to a user namespace let's make container's
root owner of sysctls affecting said network namespace instead of global
root.

This also allows us to clean up net_ctl_permissions() because we do not
need to fudge permissions anymore for the container's owner since it now
owns the objects in question.

Acked-by: "Eric W. Biederman" <ebied...@xmission.com>
Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 fs/proc/proc_sysctl.c  |  5 +
 include/linux/sysctl.h |  4 
 net/sysctl_net.c   | 29 -
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 5e57c3e..28f9085 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -430,6 +430,7 @@ static int sysctl_perm(struct ctl_table_header *head, 
struct ctl_table *table, i
 static struct inode *proc_sys_make_inode(struct super_block *sb,
struct ctl_table_header *head, struct ctl_table *table)
 {
+   struct ctl_table_root *root = head->root;
struct inode *inode;
struct proc_inode *ei;
 
@@ -457,6 +458,10 @@ static struct inode *proc_sys_make_inode(struct 
super_block *sb,
if (is_empty_dir(head))
make_empty_dir_inode(inode);
}
+
+   if (root->set_ownership)
+   root->set_ownership(head, table, >i_uid, >i_gid);
+
 out:
return inode;
 }
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index fa7bc29..55bec2f 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* For the /proc/sys support */
@@ -156,6 +157,9 @@ struct ctl_table_root {
struct ctl_table_set default_set;
struct ctl_table_set *(*lookup)(struct ctl_table_root *root,
   struct nsproxy *namespaces);
+   void (*set_ownership)(struct ctl_table_header *head,
+ struct ctl_table *table,
+ kuid_t *uid, kgid_t *gid);
int (*permissions)(struct ctl_table_header *head, struct ctl_table 
*table);
 };
 
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index ed98c1f..5bc1a3d 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -42,26 +42,37 @@ static int net_ctl_permissions(struct ctl_table_header 
*head,
   struct ctl_table *table)
 {
struct net *net = container_of(head->set, struct net, sysctls);
-   kuid_t root_uid = make_kuid(net->user_ns, 0);
-   kgid_t root_gid = make_kgid(net->user_ns, 0);
 
/* Allow network administrator to have same access as root. */
-   if (ns_capable(net->user_ns, CAP_NET_ADMIN) ||
-   uid_eq(root_uid, current_euid())) {
+   if (ns_capable(net->user_ns, CAP_NET_ADMIN)) {
int mode = (table->mode >> 6) & 7;
return (mode << 6) | (mode << 3) | mode;
}
-   /* Allow netns root group to have the same access as the root group */
-   if (in_egroup_p(root_gid)) {
-   int mode = (table->mode >> 3) & 7;
-   return (mode << 3) | mode;
-   }
+
return table->mode;
 }
 
+static void net_ctl_set_ownership(struct ctl_table_header *head,
+ struct ctl_table *table,
+ kuid_t *uid, kgid_t *gid)
+{
+   struct net *net = container_of(head->set, struct net, sysctls);
+   kuid_t ns_root_uid;
+   kgid_t ns_root_gid;
+
+   ns_root_uid = make_kuid(net->user_ns, 0);
+   if (uid_valid(ns_root_uid))
+   *uid = ns_root_uid;
+
+   ns_root_gid = make_kgid(net->user_ns, 0);
+   if (gid_valid(ns_root_gid))
+   *gid = ns_root_gid;
+}
+
 static struct ctl_table_root net_sysctl_root = {
.lookup = net_ctl_header_lookup,
.permissions = net_ctl_permissions,
+   .set_ownership = net_ctl_set_ownership,
 };
 
 static int __net_init sysctl_net_init(struct net *net)
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH] proc: make proc entries inherit ownership from parent

[Dmitry Torokhov]

On Thu, Aug 4, 2016 at 8:22 PM, Dmitry Torokhov
<dmitry.torok...@gmail.com> wrote:
> There are certain parameters that belong to net namespace and that are
> exported in /proc. They should be controllable by the container's owner,
> but are currently owned by global root and thus not available.
>
> Let's change proc code to inherit ownership of parent entry, and when
> create per-ns "net" proc entry set it up as owned by container's owner.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
> ---

Unfortunately this blows up if !CONFIG_NET_NS because of:

commit ed160e839d2e1118529e58b04d52dba703ca629c
Author: Denis V. Lunev <d...@openvz.org>
Date:   Tue Nov 13 03:23:21 2007 -0800

[NET]: Cleanup pernet operation without CONFIG_NET_NS

If CONFIG_NET_NS is not set, the only namespace is possible.

This patch removes list of pernet_operations and cleanups code a bit.
This list is not needed if there are no namespaces. We should just call
->init method.

Additionally, the ->exit will be called on module unloading only. This
case is safe - the code is not discarded. For the in/kernel code, ->exit
should never be called.

Signed-off-by: Denis V. Lunev <d...@openvz.org>
Signed-off-by: David S. Miller <da...@davemloft.net>

This causes proc_net_ns_init() to be called with not-yet-initialized
init_net namespace and oops due to net->user_ns being NULL.

Unfortunately simply reverting did not appear to work. I'll figure out
what to do and resubmit.


>  fs/proc/generic.c  |  2 ++
>  fs/proc/proc_net.c | 13 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index c633476..bca66d8 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -390,6 +390,8 @@ static struct proc_dir_entry *__proc_create(struct 
> proc_dir_entry **parent,
> atomic_set(>count, 1);
> spin_lock_init(>pde_unload_lock);
> INIT_LIST_HEAD(>pde_openers);
> +   proc_set_user(ent, (*parent)->uid, (*parent)->gid);
> +
>  out:
> return ent;
>  }
> diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
> index c8bbc68..d701738 100644
> --- a/fs/proc/proc_net.c
> +++ b/fs/proc/proc_net.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -185,6 +186,8 @@ const struct file_operations proc_net_operations = {
>  static __net_init int proc_net_ns_init(struct net *net)
>  {
> struct proc_dir_entry *netd, *net_statd;
> +   kuid_t uid;
> +   kgid_t gid;
> int err;
>
> err = -ENOMEM;
> @@ -199,6 +202,16 @@ static __net_init int proc_net_ns_init(struct net *net)
> netd->parent = _root;
> memcpy(netd->name, "net", 4);
>
> +   uid = make_kuid(net->user_ns, 0);
> +   if (!uid_valid(uid))
> +   uid = GLOBAL_ROOT_UID;
> +
> +   gid = make_kgid(net->user_ns, 0);
> +   if (!gid_valid(gid))
> +   gid = GLOBAL_ROOT_GID;
> +
> +   proc_set_user(netd, uid, gid);
> +
> err = -EEXIST;
> net_statd = proc_net_mkdir(net, "stat", netd);
> if (!net_statd)
> --
> 2.8.0.rc3.226.g39d4020
>
>
> --
> Dmitry

Thanks.

-- 
Dmitry

Re: [PATCH] net: make net namespace sysctls belong to container's owner

[Dmitry Torokhov]

On Mon, Aug 8, 2016 at 2:08 PM, Eric W. Biederman <ebied...@xmission.com> wrote:
> Dmitry Torokhov <dmitry.torok...@gmail.com> writes:
>
>> If net namespace is attached to a user namespace let's make container's
>> root owner of sysctls affecting said network namespace instead of global
>> root.
>>
>> This also allows us to clean up net_ctl_permissions() because we do not
>> need to fudge permissions anymore for the container's owner since it now
>> owns the objects in question.
>
> Acked-by: "Eric W. Biederman" <ebied...@xmission.com>
>
> Overall this seems reasonable.  However I am not a fan of your error
> handling.
>
>> Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
>> ---
>>
>> This helps when running Android CTS in a container, but I think it makes
>> sense regardless.
>
>> +static void net_ctl_set_ownership(struct ctl_table_header *head,
>> +   struct ctl_table *table,
>> +   kuid_t *uid, kgid_t *gid)
>> +{
>> + struct net *net = container_of(head->set, struct net, sysctls);
>> +
>> + *uid = make_kuid(net->user_ns, 0);
>> + if (!uid_valid(*uid))
>> + *uid = GLOBAL_ROOT_UID;
>> +
>> + *gid = make_kgid(net->user_ns, 0);
>> + if (!gid_valid(*gid))
>> + *gid = GLOBAL_ROOT_GID;
>
> This code should eiter be:
> *uid = make_kuid(net->user_ns, 0);
> *gid = make_kgid(net->user_ns, 0);
>
> Or it should be:
> tmp_uid = make_kuid(net->user_ns, 0);
> if (uid_valid(tmp_uid))
> *uid = tmp_uid;
>
> tmp_gid = make_kgid(net->user_ns, 0);
> if (gid_valid(tmp_gid))
> *gid = tmp_gid;
>
> It is just very fragile to assume to know what uid and gid
> would be if this code fails.
>
> As of v4.8-rc1 INVALID_UID and INVALID_GID can be set in inode->i_uid
> and inode->i_gid without causing horrible vfs confusion (making the
> first option viable), but I expect with the mention of Android you want
> to backport this so I will ask that you ask to implement the error
> handling that doesn't assume you know better than the generic code.
>
> If you don't have a better value to set something to it really should be
> left alone.

OK, fair enough. I will adopt the 2nd option and will resubmit. I need
to also test without net namespaces support (my other change blows up
because we are getting half-initialized init_net structure when
namespaces are disabled).

Thanks.

-- 
Dmitry

[PATCH] proc: make proc entries inherit ownership from parent

[Dmitry Torokhov]

There are certain parameters that belong to net namespace and that are
exported in /proc. They should be controllable by the container's owner,
but are currently owned by global root and thus not available.

Let's change proc code to inherit ownership of parent entry, and when
create per-ns "net" proc entry set it up as owned by container's owner.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---
 fs/proc/generic.c  |  2 ++
 fs/proc/proc_net.c | 13 +
 2 files changed, 15 insertions(+)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index c633476..bca66d8 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -390,6 +390,8 @@ static struct proc_dir_entry *__proc_create(struct 
proc_dir_entry **parent,
atomic_set(>count, 1);
spin_lock_init(>pde_unload_lock);
INIT_LIST_HEAD(>pde_openers);
+   proc_set_user(ent, (*parent)->uid, (*parent)->gid);
+
 out:
return ent;
 }
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index c8bbc68..d701738 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -185,6 +186,8 @@ const struct file_operations proc_net_operations = {
 static __net_init int proc_net_ns_init(struct net *net)
 {
struct proc_dir_entry *netd, *net_statd;
+   kuid_t uid;
+   kgid_t gid;
int err;
 
err = -ENOMEM;
@@ -199,6 +202,16 @@ static __net_init int proc_net_ns_init(struct net *net)
netd->parent = _root;
memcpy(netd->name, "net", 4);
 
+   uid = make_kuid(net->user_ns, 0);
+   if (!uid_valid(uid))
+   uid = GLOBAL_ROOT_UID;
+
+   gid = make_kgid(net->user_ns, 0);
+   if (!gid_valid(gid))
+   gid = GLOBAL_ROOT_GID;
+
+   proc_set_user(netd, uid, gid);
+
err = -EEXIST;
net_statd = proc_net_mkdir(net, "stat", netd);
if (!net_statd)
-- 
2.8.0.rc3.226.g39d4020


-- 
Dmitry

[PATCH] net: make net namespace sysctls belong to container's owner

[Dmitry Torokhov]

If net namespace is attached to a user namespace let's make container's
root owner of sysctls affecting said network namespace instead of global
root.

This also allows us to clean up net_ctl_permissions() because we do not
need to fudge permissions anymore for the container's owner since it now
owns the objects in question.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>
---

This helps when running Android CTS in a container, but I think it makes
sense regardless.

 fs/proc/proc_sysctl.c  |  5 +
 include/linux/sysctl.h |  4 
 net/sysctl_net.c   | 27 ++-
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 5e57c3e..28f9085 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -430,6 +430,7 @@ static int sysctl_perm(struct ctl_table_header *head, 
struct ctl_table *table, i
 static struct inode *proc_sys_make_inode(struct super_block *sb,
struct ctl_table_header *head, struct ctl_table *table)
 {
+   struct ctl_table_root *root = head->root;
struct inode *inode;
struct proc_inode *ei;
 
@@ -457,6 +458,10 @@ static struct inode *proc_sys_make_inode(struct 
super_block *sb,
if (is_empty_dir(head))
make_empty_dir_inode(inode);
}
+
+   if (root->set_ownership)
+   root->set_ownership(head, table, >i_uid, >i_gid);
+
 out:
return inode;
 }
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index fa7bc29..55bec2f 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* For the /proc/sys support */
@@ -156,6 +157,9 @@ struct ctl_table_root {
struct ctl_table_set default_set;
struct ctl_table_set *(*lookup)(struct ctl_table_root *root,
   struct nsproxy *namespaces);
+   void (*set_ownership)(struct ctl_table_header *head,
+ struct ctl_table *table,
+ kuid_t *uid, kgid_t *gid);
int (*permissions)(struct ctl_table_header *head, struct ctl_table 
*table);
 };
 
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index ed98c1f..ff68326 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -42,26 +42,35 @@ static int net_ctl_permissions(struct ctl_table_header 
*head,
   struct ctl_table *table)
 {
struct net *net = container_of(head->set, struct net, sysctls);
-   kuid_t root_uid = make_kuid(net->user_ns, 0);
-   kgid_t root_gid = make_kgid(net->user_ns, 0);
 
/* Allow network administrator to have same access as root. */
-   if (ns_capable(net->user_ns, CAP_NET_ADMIN) ||
-   uid_eq(root_uid, current_euid())) {
+   if (ns_capable(net->user_ns, CAP_NET_ADMIN)) {
int mode = (table->mode >> 6) & 7;
return (mode << 6) | (mode << 3) | mode;
}
-   /* Allow netns root group to have the same access as the root group */
-   if (in_egroup_p(root_gid)) {
-   int mode = (table->mode >> 3) & 7;
-   return (mode << 3) | mode;
-   }
+
return table->mode;
 }
 
+static void net_ctl_set_ownership(struct ctl_table_header *head,
+ struct ctl_table *table,
+ kuid_t *uid, kgid_t *gid)
+{
+   struct net *net = container_of(head->set, struct net, sysctls);
+
+   *uid = make_kuid(net->user_ns, 0);
+   if (!uid_valid(*uid))
+   *uid = GLOBAL_ROOT_UID;
+
+   *gid = make_kgid(net->user_ns, 0);
+   if (!gid_valid(*gid))
+   *gid = GLOBAL_ROOT_GID;
+}
+
 static struct ctl_table_root net_sysctl_root = {
.lookup = net_ctl_header_lookup,
.permissions = net_ctl_permissions,
+   .set_ownership = net_ctl_set_ownership,
 };
 
 static int __net_init sysctl_net_init(struct net *net)
-- 
2.8.0.rc3.226.g39d4020


-- 
Dmitry

Re: [PATCH v2 4/8] xen: Use the correctly the Xen memory terminologies

[Dmitry Torokhov]

On Wed, Aug 05, 2015 at 11:08:55AM +0100, Stefano Stabellini wrote:
 On Tue, 4 Aug 2015, Julien Grall wrote:
  Based on include/xen/mm.h [1], Linux is mistakenly using MFN when GFN
  is meant, I suspect this is because the first support for Xen was for
  PV. This resulted in some misimplementation of helpers on ARM and
  confused developers about the expected behavior.
  
  For instance, with pfn_to_mfn, we expect to get an MFN based on the name.
  Although, if we look at the implementation on x86, it's returning a GFN.
  
  For clarity and avoid new confusion, replace any reference to mfn with
  gfn in any helpers used by PV drivers. The x86 code will still keep some
  reference of pfn_to_mfn but exclusively for PV (a BUG_ON has been added
  to ensure this). No changes as been made in the hypercall field, even
  though they may be invalid, in order to keep the same as the defintion
  in xen repo.
  
  Take also the opportunity to simplify simple construction such
  as pfn_to_mfn(page_to_pfn(page)) into page_to_gfn. More complex clean up
  will come in follow-up patches.
  
  [1] 
  http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e758ed14f390342513405dd766e874934573e6cb
  
  Signed-off-by: Julien Grall julien.gr...@citrix.com
  Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com
  Cc: Russell King li...@arm.linux.org.uk
  Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
  Cc: Boris Ostrovsky boris.ostrov...@oracle.com
  Cc: David Vrabel david.vra...@citrix.com
  Cc: Thomas Gleixner t...@linutronix.de
  Cc: Ingo Molnar mi...@redhat.com
  Cc: H. Peter Anvin h...@zytor.com
  Cc: x...@kernel.org
  Cc: Roger Pau Monné roger@citrix.com
  Cc: Dmitry Torokhov dmitry.torok...@gmail.com
  Cc: Ian Campbell ian.campb...@citrix.com
  Cc: Wei Liu wei.l...@citrix.com
  Cc: Juergen Gross jgr...@suse.com
  Cc: James E.J. Bottomley jbottom...@odin.com
  Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
  Cc: Jiri Slaby jsl...@suse.com
  Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
  Cc: Tomi Valkeinen tomi.valkei...@ti.com
  Cc: linux-in...@vger.kernel.org
  Cc: netdev@vger.kernel.org
  Cc: linux-s...@vger.kernel.org
  Cc: linuxppc-...@lists.ozlabs.org
  Cc: linux-fb...@vger.kernel.org
  Cc: linux-arm-ker...@lists.infradead.org
 
 Aside from the x86 bits:
 
 Reviewed-by: Stefano Stabellini stefano.stabell...@eu.citrix.com

Not really important, but just in case anyone waits for my ack on input
bits:

Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] nf: IDLETIMER: fix lockdep warning

[Dmitry Torokhov]

On Mon, Jul 13, 2015 at 6:20 AM, Pablo Neira Ayuso pa...@netfilter.org wrote:
 On Thu, Jul 09, 2015 at 05:15:01PM -0700, Dmitry Torokhov wrote:
 Dynamically allocated sysfs attributes should be initialized with
 sysfs_attr_init() otherwise lockdep will be angry with us:

 [   45.468653] BUG: key ffc030fad4e0 not in .data!
 [   45.468655] [ cut here ]
 [   45.468666] WARNING: CPU: 0 PID: 1176 at 
 /mnt/host/source/src/third_party/kernel/v3.18/kernel/locking/lockdep.c:2991 
 lockdep_init_map+0x12c/0x490()
 [   45.468672] DEBUG_LOCKS_WARN_ON(1)
 [   45.468672] CPU: 0 PID: 1176 Comm: iptables Tainted: G U  W 3.18.0 #43
 [   45.468674] Hardware name: XXX
 [   45.468675] Call trace:
 [   45.468680] [ffc0002072b4] dump_backtrace+0x0/0x10c
 [   45.468683] [ffc0002073d0] show_stack+0x10/0x1c
 [   45.468688] [ffc000a86cd4] dump_stack+0x74/0x94
 [   45.468692] [ffc000217ae0] warn_slowpath_common+0x84/0xb0
 [   45.468694] [ffc000217b84] warn_slowpath_fmt+0x4c/0x58
 [   45.468697] [ffc0002530a4] lockdep_init_map+0x128/0x490
 [   45.468701] [ffc000367ef0] __kernfs_create_file+0x80/0xe4
 [   45.468704] [ffc00036862c] sysfs_add_file_mode_ns+0x104/0x170
 [   45.468706] [ffc00036870c] sysfs_create_file_ns+0x58/0x64
 [   45.468711] [ffc000930430] idletimer_tg_checkentry+0x14c/0x324
 [   45.468714] [ffc00092a728] xt_check_target+0x170/0x198
 [   45.468717] [ffc000993efc] check_target+0x58/0x6c
 [   45.468720] [ffc000994c64] translate_table+0x30c/0x424
 [   45.468723] [ffc00099529c] do_ipt_set_ctl+0x144/0x1d0
 [   45.468728] [ffc0009079f0] nf_setsockopt+0x50/0x60
 [   45.468732] [ffc000946870] ip_setsockopt+0x8c/0xb4
 [   45.468735] [ffc0009661c0] raw_setsockopt+0x10/0x50
 [   45.468739] [ffc0008c1550] sock_common_setsockopt+0x14/0x20
 [   45.468742] [ffc0008bd190] SyS_setsockopt+0x88/0xb8
 [   45.468744] ---[ end trace 41d156354d18c039 ]---

 Applied, thanks.

 One question:

 Change-Id: I1da5cd96fc8e1e1e4209e81eba1165a42d4d45e9

 BTW, does this gerrit change ID provide any public information? Thanks.

Argh, I am sorry, I forgot to clean this out when mailing the patch.
In this particular case you can find the change in AOSP gerrit at
https://android-review.googlesource.com but without such context this
change-id is of course useless.

Thanks,
Dmitry
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] nf: IDLETIMER: fix lockdep warning

[Dmitry Torokhov]

Dynamically allocated sysfs attributes should be initialized with
sysfs_attr_init() otherwise lockdep will be angry with us:

[   45.468653] BUG: key ffc030fad4e0 not in .data!
[   45.468655] [ cut here ]
[   45.468666] WARNING: CPU: 0 PID: 1176 at 
/mnt/host/source/src/third_party/kernel/v3.18/kernel/locking/lockdep.c:2991 
lockdep_init_map+0x12c/0x490()
[   45.468672] DEBUG_LOCKS_WARN_ON(1)
[   45.468672] CPU: 0 PID: 1176 Comm: iptables Tainted: G U  W 3.18.0 #43
[   45.468674] Hardware name: XXX
[   45.468675] Call trace:
[   45.468680] [ffc0002072b4] dump_backtrace+0x0/0x10c
[   45.468683] [ffc0002073d0] show_stack+0x10/0x1c
[   45.468688] [ffc000a86cd4] dump_stack+0x74/0x94
[   45.468692] [ffc000217ae0] warn_slowpath_common+0x84/0xb0
[   45.468694] [ffc000217b84] warn_slowpath_fmt+0x4c/0x58
[   45.468697] [ffc0002530a4] lockdep_init_map+0x128/0x490
[   45.468701] [ffc000367ef0] __kernfs_create_file+0x80/0xe4
[   45.468704] [ffc00036862c] sysfs_add_file_mode_ns+0x104/0x170
[   45.468706] [ffc00036870c] sysfs_create_file_ns+0x58/0x64
[   45.468711] [ffc000930430] idletimer_tg_checkentry+0x14c/0x324
[   45.468714] [ffc00092a728] xt_check_target+0x170/0x198
[   45.468717] [ffc000993efc] check_target+0x58/0x6c
[   45.468720] [ffc000994c64] translate_table+0x30c/0x424
[   45.468723] [ffc00099529c] do_ipt_set_ctl+0x144/0x1d0
[   45.468728] [ffc0009079f0] nf_setsockopt+0x50/0x60
[   45.468732] [ffc000946870] ip_setsockopt+0x8c/0xb4
[   45.468735] [ffc0009661c0] raw_setsockopt+0x10/0x50
[   45.468739] [ffc0008c1550] sock_common_setsockopt+0x14/0x20
[   45.468742] [ffc0008bd190] SyS_setsockopt+0x88/0xb8
[   45.468744] ---[ end trace 41d156354d18c039 ]---

Change-Id: I1da5cd96fc8e1e1e4209e81eba1165a42d4d45e9
Signed-off-by: Dmitry Torokhov d...@google.com
---
 net/netfilter/xt_IDLETIMER.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c
index 31f76cd..0975c993 100644
--- a/net/netfilter/xt_IDLETIMER.c
+++ b/net/netfilter/xt_IDLETIMER.c
@@ -286,6 +286,7 @@ static int idletimer_tg_create(struct idletimer_tg_info 
*info)
goto out;
}
 
+   sysfs_attr_init(info-timer-attr.attr);
info-timer-attr.attr.name = kstrdup(info-label, GFP_KERNEL);
if (!info-timer-attr.attr.name) {
ret = -ENOMEM;
-- 
2.4.3.573.g4eafbef


-- 
Dmitry
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Remove pointless casts from void pointers,

[Dmitry Torokhov]

On 10/26/07, Jeff Garzik [EMAIL PROTECTED] wrote:
  drivers/input/touchscreen/h3600_ts_input.c |4 ++--

Acked-by: Dmitry Torokhov [EMAIL PROTECTED]

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux kernel networking code review request

[Dmitry Torokhov]

On Friday 19 October 2007, Templin, Fred L wrote:
 Hello,
 
 I would like to get a review on some new Linux
 kernel networking code. The code is based on the
 2.6.23 source tree and touches three files - two
 in ./net/ipv6 and one in ./include/net. What is
 the proper procedure for requesting a review?
 

Just don your flameproof pants and post the changes here ;)

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 0/3] rfkill

[Dmitry Torokhov]

Hi Ivo,

On Saturday 08 September 2007 11:10, Ivo van Doorn wrote:
 Hi Dmitry,
 
 I have a few rfkill related patches for which I would prefer if you to could 
 take a look at before I send them for inclusion.
 

Looks good, feel free to add

Acked-by: Dmitry Torokhov [EMAIL PROTECTED]

Thanks!

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Merge the Sonics Silicon Backplane subsystem

[Dmitry Torokhov]

On Friday 27 July 2007 16:12, Andrew Morton wrote:
 On Fri, 27 Jul 2007 21:43:59 +0200
 Michael Buesch [EMAIL PROTECTED] wrote:
 
   Sure, but why is the locking interruptible rather than plain old
   mutex_lock()?
  
  Hm, well. We hold this mutex for several seconds, as writing takes
  this long. So I simply thought it was worth allowing the waiter
  to interrupt here. If you say that's not an issue, I'll be happy
  to use mutex_lock() and reduce code complexity in this area.
 
 So..  is that what the _interruptible() is for?  To allow an impatient user 
 to ^c
 a read?
 
 If so, that sounds reasonable.  It's worth a comment explaining these 
 decisions
 to future readers, because it is hard to work out this sort of thinking just
 from the bare C code.

I think most of sysfs -show() and -store() implementations use
_interruptible() variant to allow user to interrupt and return early.

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] RF Kill

[Dmitry Torokhov]

On 4/26/07, David Miller [EMAIL PROTECTED] wrote:


Actually, I'm reverting this, sorry.

Besides the locking and other issues I pointed out, this
thing won't even link.  Please do not test the build with
various configurations when submitting new code like this.

Thanks.

 MODPOST 139 modules
WARNING: input_register_handle [net/rfkill/rfkill-input.ko] undefined!
WARNING: input_unregister_handle [net/rfkill/rfkill-input.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2



Argh, I forgot that this part of input core changes is not in mainline
yet. I am very sorry.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] RF Kill

[Dmitry Torokhov]

On 4/26/07, David Miller [EMAIL PROTECTED] wrote:

From: Dmitry Torokhov [EMAIL PROTECTED]
Date: Tue, 10 Apr 2007 02:29:31 -0400

 On Tuesday 10 April 2007 01:58, Dmitry Torokhov wrote:
  Hi,
 
  This is a modified version of rfkill patch that provides infrastructure
  for controlling state of RF transmitters found on various cards.

 Well, Andrew found bunch of issues with the patch so here is an
 updated version...

Patch applied, although one part of the locking is slightly
suspect:

 +static void rfkill_task_handler(struct work_struct *work)
 +{
 + struct rfkill_task *task = container_of(work, struct rfkill_task, work);
 + enum rfkill_state state;
 +
 + mutex_lock(task-mutex);
 +
 + spin_lock_irq(task-lock);
 + state = task-desired_state;
 + spin_unlock_irq(task-lock);
 +
 + if (state != task-current_state) {
 + rfkill_switch_all(task-type, state);
 + task-current_state = state;
 + }
 +
 + mutex_unlock(task-mutex);
 +}

I applied this, but...

That lock around the read doesn't make any sense, reads
are atomic on all SMP processors. You're not going to
see a partial word-update if you take away that lock so
it isn't doing anything.



Ah, OK. I was always concerned with partial word updates but if we
have this guarantee this makes things simplier. I will remove the lock
there and I will add a comment that a temp is still needed since
desired_state may be changed from other thread.


If locking is really needed here, it probably need to protect
the whole read-modify-write operation transferring the
desired_state to the current_state.

In another code block, this -desired_state thing is
treated like a boolean instead of the enumeration that
it is supposed to be:


It is boolean enumeration with values of 0 and 1 ;) and so state =
!state should work.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] RF Kill

[Dmitry Torokhov]

On Tuesday 10 April 2007 01:58, Dmitry Torokhov wrote:
 Hi,
 
 This is a modified version of rfkill patch that provides infrastructure
 for controlling state of RF transmitters found on various cards.

Well, Andrew found bunch of issues with the patch so here is an
updated version...

-- 
Dmitry


From: Ivo van Doorn [EMAIL PROTECTED]

rfkill - Add support for input key to control wireless radio

Signed-off-by Ivo van Doorn [EMAIL PROTECTED]
Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 include/linux/rfkill.h|   87 +
 net/Kconfig   |1 
 net/Makefile  |1 
 net/rfkill/Kconfig|   24 ++
 net/rfkill/Makefile   |6 
 net/rfkill/rfkill-input.c |  170 +++
 net/rfkill/rfkill.c   |  406 ++
 7 files changed, 695 insertions(+)

Index: work/include/linux/rfkill.h
===
--- /dev/null
+++ work/include/linux/rfkill.h
@@ -0,0 +1,87 @@
+#ifndef __RFKILL_H
+#define __RFKILL_H
+
+/*
+ * Copyright (C) 2006 Ivo van Doorn
+ * Copyright (C) 2007 Dmitry Torokhov
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the
+ * Free Software Foundation, Inc.,
+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include linux/list.h
+#include linux/mutex.h
+#include linux/device.h
+
+/**
+ * enum rfkill_type - type of rfkill switch.
+ * RFKILL_TYPE_WLAN: switch is no a Wireless network devices.
+ * RFKILL_TYPE_BlUETOOTH: switch is on a bluetooth device.
+ * RFKILL_TYPE_IRDA: switch is on an infrared devices.
+ */
+enum rfkill_type {
+   RFKILL_TYPE_WLAN = 0,
+   RFKILL_TYPE_BLUETOOTH = 1,
+   RFKILL_TYPE_IRDA = 2,
+   RFKILL_TYPE_MAX = 3,
+};
+
+enum rfkill_state {
+   RFKILL_STATE_OFF= 0,
+   RFKILL_STATE_ON = 1,
+};
+
+/**
+ * struct rfkill - rfkill control structure.
+ * @name: Name of the switch.
+ * @type: Radio type which the button controls, the value stored
+ * here should be a value from enum rfkill_type.
+ * @state: State of the switch (on/off).
+ * @user_claim: Set when the switch is controlled exlusively by userspace.
+ * @mutex: Guards switch state transitions
+ * @data: Pointer to the RF button drivers private data which will be
+ * passed along when toggling radio state.
+ * @toggle_radio(): Mandatory handler to control state of the radio.
+ * @dev: Device structure integrating the switch into device tree.
+ * @node: Used to place switch into list of all switches known to the
+ * the system.
+ *
+ * This structure represents a RF switch located on a network device.
+ */
+struct rfkill {
+   char *name;
+   enum rfkill_type type;
+
+   enum rfkill_state state;
+   bool user_claim;
+
+   struct mutex mutex;
+
+   void *data;
+   int (*toggle_radio)(void *data, enum rfkill_state state);
+
+   struct device dev;
+   struct list_head node;
+};
+#define to_rfkill(d)   container_of(d, struct rfkill, dev)
+
+struct rfkill *rfkill_allocate(struct device *parent, enum rfkill_type type);
+void rfkill_free(struct rfkill *rfkill);
+int rfkill_register(struct rfkill *rfkill);
+void rfkill_unregister(struct rfkill *rfkill);
+
+void rfkill_switch_all(enum rfkill_type type, enum rfkill_state state);
+
+#endif /* RFKILL_H */
Index: work/net/Kconfig
===
--- work.orig/net/Kconfig
+++ work/net/Kconfig
@@ -220,6 +220,7 @@ source net/ax25/Kconfig
 source net/irda/Kconfig
 source net/bluetooth/Kconfig
 source net/ieee80211/Kconfig
+source net/rfkill/Kconfig
 
 config WIRELESS_EXT
bool
Index: work/net/Makefile
===
--- work.orig/net/Makefile
+++ work/net/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_IEEE80211)   += ieee80211/
 obj-$(CONFIG_TIPC) += tipc/
 obj-$(CONFIG_NETLABEL) += netlabel/
 obj-$(CONFIG_IUCV) += iucv/
+obj-$(CONFIG_RFKILL)   += rfkill/
 
 ifeq ($(CONFIG_NET),y)
 obj-$(CONFIG_SYSCTL)   += sysctl_net.o
Index: work/net/rfkill/Kconfig
===
--- /dev/null
+++ work/net/rfkill/Kconfig
@@ -0,0 +1,24 @@
+#
+# RF switch subsystem configuration
+#
+menuconfig RFKILL
+   tristate RF switch subsystem support
+   help

[RFC] RF Kill

[Dmitry Torokhov]

Hi,

This is a modified version of rfkill patch that provides infrastructure
for controlling state of RF transmitters found on various cards. It
works as follows:

Network card drivers that are able to control state of their RF
transmitters allocate and register rfkill structures. Every rfkill
structure belongs to one of RF classes (WLAN, Bluetooth or IRDA)
and exports its name, type, state and user_claim through sysfs.
Drivers implementing RF switches must supply toggle_radio() method
for controlling switch.  

There is also an input handler that catches KEY_WLAN and KEY_BLUETOOTH
events passing through input system and toggles state of all RF
switches of appropriate type registered with rfkill system (unless
a switch was claimed by userspace in which case it is left alone).
I think this provides basic functionality that most of the users need
and any advanced control will have to be done from userspace.

Creation of input devices that emit KEY_WLAN and KEY_BLUETOOTH is
outside of scope of rfkill infrastructure but input core will
provide a skeleton for easy implementation of simple polled buttons.

-- 
Dmitry


From: Ivo van Doorn [EMAIL PROTECTED]

rfkill - Add support for input key to control wireless radio

Signed-off-by Ivo van Doorn [EMAIL PROTECTED]
Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 include/linux/rfkill.h|   85 +
 net/Kconfig   |1 
 net/Makefile  |1 
 net/rfkill/Kconfig|   24 ++
 net/rfkill/Makefile   |6 
 net/rfkill/rfkill-input.c |  169 +++
 net/rfkill/rfkill.c   |  400 ++
 7 files changed, 686 insertions(+)

Index: work/include/linux/rfkill.h
===
--- /dev/null
+++ work/include/linux/rfkill.h
@@ -0,0 +1,85 @@
+#ifndef __RFKILL_H
+#define __RFKILL_H
+
+/*
+ * Copyright (C) 2006 Ivo van Doorn
+ * Copyright (C) 2007 Dmitry Torokhov
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the
+ * Free Software Foundation, Inc.,
+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include linux/device.h
+
+/**
+ * enum rfkill_type - type of rfkill switch.
+ * RFKILL_TYPE_WLAN: switch is no a Wireless network devices.
+ * RFKILL_TYPE_BlUETOOTH: switch is on a bluetooth device.
+ * RFKILL_TYPE_IRDA: switch is on an infrared devices.
+ */
+enum rfkill_type {
+   RFKILL_TYPE_WLAN = 0,
+   RFKILL_TYPE_BLUETOOTH = 1,
+   RFKILL_TYPE_IRDA = 2,
+   RFKILL_TYPE_MAX = 3,
+};
+
+enum rfkill_state {
+   RFKILL_STATE_OFF= 0,
+   RFKILL_STATE_ON = 1,
+};
+
+/**
+ * struct rfkill - rfkill control structure.
+ * @name: Name of the switch.
+ * @type: Radio type which the button controls, the value stored
+ * here should be a value from enum rfkill_type.
+ * @state: State of the switch (on/off).
+ * @user_claim: Set when the switch is controlled exlusively by userspace.
+ * @mutex: Guards switch state transitions
+ * @data: Pointer to the RF button drivers private data which will be
+ * passed along when toggling radio state.
+ * @toggle_radio(): Mandatory handler to control state of the radio.
+ * @dev: Device structure integrating the switch into device tree.
+ * @node: Used to place switch into list of all switches known to the
+ * the system.
+ *
+ * This structure represents a RF switch located on a network device.
+ */
+struct rfkill {
+   char *name;
+   enum rfkill_type type;
+
+   enum rfkill_state state;
+   bool user_claim;
+
+   struct mutex mutex;
+
+   void *data;
+   int (*toggle_radio)(void *data, enum rfkill_state state);
+
+   struct device dev;
+   struct list_head node;
+};
+#define to_rfkill(d)   container_of(d, struct rfkill, dev)
+
+struct rfkill *rfkill_allocate(struct device *parent, enum rfkill_type type);
+void rfkill_free(struct rfkill *rfkill);
+int rfkill_register(struct rfkill *rfkill);
+void rfkill_unregister(struct rfkill *rfkill);
+
+void rfkill_switch_all(enum rfkill_type type, enum rfkill_state state);
+
+#endif /* RFKILL_H */
Index: work/net/Kconfig
===
--- work.orig/net/Kconfig
+++ work/net/Kconfig
@@ -220,6 +220,7 @@ source net/ax25/Kconfig
 source net/irda/Kconfig
 source net/bluetooth/Kconfig
 source net/ieee80211/Kconfig
+source net/rfkill

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On Saturday 31 March 2007 08:49, Ivo van Doorn wrote:
 
 Well that would mean rfkill would be ready to applied to one of the kernel 
 trees right? :)

Well, that would be up to that particular tree maintainer. I am not sure
who maintains the net/... David Miller maybe? Anyway, below is the same
rfkill patch that I posted before except that everything is moved in
net/ instead of being in drivers/input/misc. 

I will make sure that input-polldev will get into input three tough.
 
-- 
Dmitry

From: Ivo van Doorn [EMAIL PROTECTED]

Input: rfkill - Add support for input key to control wireless radio

Signed-off-by Ivo van Doorn [EMAIL PROTECTED]
Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 include/linux/rfkill.h|   85 +
 net/Kconfig   |1 
 net/Makefile  |1 
 net/rfkill/Kconfig|   24 ++
 net/rfkill/Makefile   |6 
 net/rfkill/rfkill-input.c |  169 +++
 net/rfkill/rfkill.c   |  400 ++
 7 files changed, 686 insertions(+)

Index: work/include/linux/rfkill.h
===
--- /dev/null
+++ work/include/linux/rfkill.h
@@ -0,0 +1,85 @@
+#ifndef __RFKILL_H
+#define __RFKILL_H
+
+/*
+ * Copyright (C) 2006 Ivo van Doorn
+ * Copyright (C) 2007 Dmitry Torokhov
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the
+ * Free Software Foundation, Inc.,
+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include linux/device.h
+
+/**
+ * enum rfkill_type - type of rfkill switch.
+ * RFKILL_TYPE_WLAN: switch is no a Wireless network devices.
+ * RFKILL_TYPE_BlUETOOTH: switch is on a bluetooth device.
+ * RFKILL_TYPE_IRDA: switch is on an infrared devices.
+ */
+enum rfkill_type {
+   RFKILL_TYPE_WLAN = 0,
+   RFKILL_TYPE_BLUETOOTH = 1,
+   RFKILL_TYPE_IRDA = 2,
+   RFKILL_TYPE_MAX = 3,
+};
+
+enum rfkill_state {
+   RFKILL_STATE_OFF= 0,
+   RFKILL_STATE_ON = 1,
+};
+
+/**
+ * struct rfkill - rfkill control structure.
+ * @name: Name of the switch.
+ * @type: Radio type which the button controls, the value stored
+ * here should be a value from enum rfkill_type.
+ * @state: State of the switch (on/off).
+ * @user_claim: Set when the switch is controlled exlusively by userspace.
+ * @mutex: Guards switch state transitions
+ * @data: Pointer to the RF button drivers private data which will be
+ * passed along when toggling radio state.
+ * @toggle_radio(): Mandatory handler to control state of the radio.
+ * @dev: Device structure integrating the switch into device tree.
+ * @node: Used to place switch into list of all switches known to the
+ * the system.
+ *
+ * This structure represents a RF switch located on a network device.
+ */
+struct rfkill {
+   char *name;
+   enum rfkill_type type;
+
+   enum rfkill_state state;
+   bool user_claim;
+
+   struct mutex mutex;
+
+   void *data;
+   int (*toggle_radio)(void *data, enum rfkill_state state);
+
+   struct device dev;
+   struct list_head node;
+};
+#define to_rfkill(d)   container_of(d, struct rfkill, dev)
+
+struct rfkill *rfkill_allocate(struct device *parent, enum rfkill_type type);
+void rfkill_free(struct rfkill *rfkill);
+int rfkill_register(struct rfkill *rfkill);
+void rfkill_unregister(struct rfkill *rfkill);
+
+void rfkill_switch_all(enum rfkill_type type, enum rfkill_state state);
+
+#endif /* RFKILL_H */
Index: work/net/Kconfig
===
--- work.orig/net/Kconfig
+++ work/net/Kconfig
@@ -220,6 +220,7 @@ source net/ax25/Kconfig
 source net/irda/Kconfig
 source net/bluetooth/Kconfig
 source net/ieee80211/Kconfig
+source net/rfkill/Kconfig
 
 config WIRELESS_EXT
bool
Index: work/net/Makefile
===
--- work.orig/net/Makefile
+++ work/net/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_IEEE80211)   += ieee80211/
 obj-$(CONFIG_TIPC) += tipc/
 obj-$(CONFIG_NETLABEL) += netlabel/
 obj-$(CONFIG_IUCV) += iucv/
+obj-$(CONFIG_RFKILL)   += rfkill/
 
 ifeq ($(CONFIG_NET),y)
 obj-$(CONFIG_SYSCTL)   += sysctl_net.o
Index: work/net/rfkill/Kconfig
===
--- /dev/null
+++ work/net

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 3/30/07, Ivo van Doorn [EMAIL PROTECTED] wrote:

Hi,

 I am very sorry for taking so much time to respond but finally I went
 through the patch and I still have the same objection as before -
 it mixes two logically (and often physically) separated objects into
 a single entity. I think that RF switch and button should be separate
 entities, created and destroyed independently of each other. This way
 everything handled uniformly by the kernel.

ok. Sounds good as well. :)

 I attempted to rework the rfkill core supprt and simplify it and
 came up with the patch below. Network card drivers that are able
 to control state of their RF transmitters allocate and register
 rfkill structures. Every rfkill structure belongs to one of
 RF classes (WLAN, Bluetooth or IRDA) and exports its name, type,
 state and user_claim through sysfs.

 There is also an input handler that catces KEY_WLAN and KEY_BLUETOOTH
 events passing through input system and toggles state of all RF
 switches of appropriate type registered with rfkill system (unless
 a switch was claimed by userspace in which case it is left alone).
 I think this provides basic functionality that most of the users need
 and any advanced control will have to be done from userspace.

Shouldn't a KEY_IRDA be added as well?
It isn't currently defined in input.h yet, but perhaps it actually should?
Or is IRDA treated differently for a specific reason?


Do we actually have cards with IRDA switches? We are kind of tight in
input KEY_ namespace so I am hesitant to add defines before there
actual users.



 In a followup patch there is a skeleton code for creating polled
 input devices. For cards that have button physically connected
 their drivers will have to register a separate input device and
 let either input handler or userspace application take care of
 switching RF state appropriately.

 My only concern is where rfkill code should live. Since it is no
 longer dependent on input core (embedded systems might disable
 rfkill-input and use bare rfkill and control state from userspace)
 it does not need to live in drivers/input.

How about:
rfkill - drivers/misc


Not in net?


rfkill_input - input/misc


I can go both ways on this one as it crosses line between input and
rfkill. I think from user/Kconfig point of view it is better to keep
it together with rfkill: user woudl select rfkill option and right
then and there decide if he wants to give the kernel ability to
automatically control RF switche


input_polldev - lib/ (perhaps small namechange to rfkill_polldev?)


No, I do not want to change name - I have bunch of drivers that can me
converted to use this skeleton - wistron, aaedkbd, hdaps, ams,
cobalt_btns. It is also pure input-related function so it is the only
module that definitely belongs to drivers/input/misc.


It would scatter the components a bit, but since each of those modules
has its own specific task this would make the most sense.
And by setting the depends and select fields for the menu entries correctly
it shouldn't be too big of a problem.

 Please let me know what you think.

Just to get it straight on how these 3 modules would cooperate (before I start 
mixing things up) ;)

 - rfkill
   - Drivers register to the rfkill module, rfkill
   - Provides the sysfs interface
   - Drivers that don't require polling should report the events to this 
module


Not quite. Drivers that have buttons do not require polling still
should create an input device and register it with input layer. Except
that with interrupt-driven devices there is not much you can stub out
so you just have to do input_allocate_device/input_register_device.



 - rfkill_input
   - Provides input device visible to userspace



No, rfkill-input is not an input device but input handler (or input
interface). It routes input events from buttons into switches (see
below).


 - input_polldev
   - Handlers polling, where the driver should check what the previous 
state was and the driver
 should send the event to rfkill.


This is the input device visible to userspace and kernel. It polls
state of the button and sends KEY_WLAN/KEY_BLUETOOTH events through
input layer. They get distributed through various input handlers such
as evdev and rfkill-input. Evdev provides route for events to
userspace where application can listen to events and then toggle RF
switches according to the current policy via
/sys/class/rfkill/rfkillXXX/state. Rfkill-input provides in-kernel
route for events into rfkill system. If rfkill-input is loaded
switches that are not claimed by userspace will be toggled
automatically.

Does this make sense?

Note that we don't start polling the state of button until tare are
users of that input device. If rfkill-input is loaded then there is a
user right away. Otherwise we wait for userspace to open evdev node.



personally I would prefer enforcing drivers to call
allocate()
register()
unregister()
free()

Especially with unregister() doing 

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 3/30/07, Ivo van Doorn [EMAIL PROTECTED] wrote:

  
   My only concern is where rfkill code should live. Since it is no
   longer dependent on input core (embedded systems might disable
   rfkill-input and use bare rfkill and control state from userspace)
   it does not need to live in drivers/input.
 
  How about:
  rfkill - drivers/misc

 Not in net?

Well in drivers/net are the network drivers but not the irda and bluetooth 
drivers,
those are located in different folders in drivers/ so I think misc would be the 
most
suitable location.


We could also consider the ./net itself. rfkill is not a driver, it is
a facility.


   - input_polldev
 - Handlers polling, where the driver should check what the previous 
state was and the driver
   should send the event to rfkill.

 This is the input device visible to userspace and kernel. It polls
 state of the button and sends KEY_WLAN/KEY_BLUETOOTH events through
 input layer. They get distributed through various input handlers such
 as evdev and rfkill-input. Evdev provides route for events to
 userspace where application can listen to events and then toggle RF
 switches according to the current policy via
 /sys/class/rfkill/rfkillXXX/state. Rfkill-input provides in-kernel
 route for events into rfkill system. If rfkill-input is loaded
 switches that are not claimed by userspace will be toggled
 automatically.

 Does this make sense?

Yes, but what if the user loads both modules or has them both compiled in?
Shouldn't there be some protection against that, since both handlers should not
be active at the same time.


Why not? evdev is just a delivery transport for input events to
userspace. Even if user wants the kernel to control state of RF
switches (which I expect most users woudl want) there still could be
applications interested in knowing that used turned off wireless. And
if userspace really wishes to control switch all by itself it can
claim it.

I guess what you are missing is that input event generated by a device
is pushed through every handler bound to the device so there is no way
for a wrong handler to steals an event from right handler. They
all work simultaneously.


  personally I would prefer enforcing drivers to call
  allocate()
  register()
  unregister()
  free()
 
  Especially with unregister() doing the same steps as free() (put_device)
  might be somwhat confusing. But might be just me. ;)
 

 I know but for refcounted objects you can't really tell when they will
 actually be freed. It depends when their last user drops off.

Then perhaps rfkill_register could call put_device() when it fails, and free()
can be removed entirely. That way it would we prevent some driver
to call free() anyway.



That would make error handling in -probe() methods a bit unwieldy I
think - if you are using the standard goto err_xxx; goto err_yyy;
technique then you have to conditionally call rfkill_free(). Hovewer
if register simply fails and does not free anything and you call
rfkill_register() last (which you need to do because driver has to be
almost fully functional to be able to serve toggle_radio calls) you
can always call rfkill_free() if something fails.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

Input: polled device skeleton

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---
 drivers/input/misc/Kconfig |   11 ++
 drivers/input/misc/Makefile|1 
 drivers/input/misc/input-polldev.c |  149 +
 include/linux/input-polldev.h  |   46 +++
 4 files changed, 207 insertions(+)

Index: linux/drivers/input/misc/Kconfig
===
--- linux.orig/drivers/input/misc/Kconfig
+++ linux/drivers/input/misc/Kconfig
@@ -81,6 +81,17 @@ config INPUT_UINPUT
  To compile this driver as a module, choose M here: the
  module will be called uinput.
 
+config INPUT_POLLDEV
+   tristate Polled input device skeleton
+   help
+ Say Y here if you are using a driver for an input
+ device that periodically polls hardware state. This
+ option is only useful for out-of-tree drivers since
+ in-tree drivers select it automatically.
+
+ To compile this driver as a module, choose M here: the
+ module will be called input-polldev.
+
 config HP_SDC_RTC
tristate HP SDC Real Time Clock   
depends on GSC || HP300
Index: linux/drivers/input/misc/Makefile
===
--- linux.orig/drivers/input/misc/Makefile
+++ linux/drivers/input/misc/Makefile
@@ -4,6 +4,7 @@
 
 # Each configuration option enables a list of files.
 
+obj-$(CONFIG_INPUT_POLLDEV)+= input-polldev.o
 obj-$(CONFIG_INPUT_SPARCSPKR)  += sparcspkr.o
 obj-$(CONFIG_INPUT_PCSPKR) += pcspkr.o
 obj-$(CONFIG_INPUT_M68K_BEEP)  += m68kspkr.o
Index: linux/drivers/input/misc/input-polldev.c
===
--- /dev/null
+++ linux/drivers/input/misc/input-polldev.c
@@ -0,0 +1,149 @@
+/*
+ * Generic implementation of a polled input device
+
+ * Copyright (c) 2007 Dmitry Torokhov
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include linux/jiffies.h
+#include linux/input-polldev.h
+
+static DEFINE_MUTEX(polldev_mutex);
+static int polldev_users;
+static struct workqueue_struct *polldev_wq;
+
+static void input_polled_device_work(struct work_struct *work)
+{
+   struct input_polled_dev *dev =
+   container_of(work, struct input_polled_dev, work.work);
+
+   dev-poll(dev);
+   queue_delayed_work(polldev_wq, dev-work,
+  msecs_to_jiffies(dev-poll_interval));
+}
+
+static int input_open_polled_device(struct input_dev *input)
+{
+   struct input_polled_dev *dev = input-private;
+   int retval;
+
+   retval = mutex_lock_interruptible(polldev_mutex);
+   if (retval)
+   return retval;
+
+   if (!polldev_users++) {
+   polldev_wq = create_singlethread_workqueue(ipolldevd);
+   if (!polldev_wq) {
+   printk(KERN_ERR input-polldev: failed to create 
+   ipolldevd workqueue\n);
+   goto out;
+   }
+   }
+
+   if (dev-flush)
+   dev-flush(dev);
+   queue_delayed_work(polldev_wq, dev-work,
+  msecs_to_jiffies(dev-poll_interval));
+
+ out:
+   mutex_unlock(polldev_mutex);
+   return retval;
+}
+
+static void input_close_polled_device(struct input_dev *input)
+{
+   struct input_polled_dev *dev = input-private;
+
+   cancel_rearming_delayed_workqueue(polldev_wq, dev-work);
+
+   mutex_lock(polldev_mutex);
+   if (!--polldev_users)
+   destroy_workqueue(polldev_wq);
+   mutex_unlock(polldev_mutex);
+}
+
+/**
+ * input_allocate_polled_device - allocated memory polled device
+ *
+ * The function allocates memory for a polled device and also
+ * for an input device associated with this polled device.
+ */
+struct input_polled_dev *input_allocate_polled_device(void)
+{
+   struct input_polled_dev *dev;
+
+   dev = kzalloc(sizeof(struct input_polled_dev), GFP_KERNEL);
+   if (!dev)
+   return NULL;
+
+   dev-input = input_allocate_device();
+   if (!dev-input) {
+   kfree(dev);
+   return NULL;
+   }
+
+   return dev;
+}
+EXPORT_SYMBOL(input_allocate_polled_device);
+
+/**
+ * input_free_polled_device - free memory allocated for polled device
+ * @dev: device to free
+ *
+ * The function frees memory allocated for pollign device and drops
+ * reference to the associated input device (if present).
+ */
+void input_free_polled_device(struct input_polled_dev *dev)
+{
+   if (dev) {
+   input_free_device(dev-input);
+   kfree(dev);
+   }
+}
+EXPORT_SYMBOL(input_free_polled_device);
+
+/**
+ * input_register_polled_device - register polled device

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On Wednesday 31 January 2007 06:20, Ivo van Doorn wrote:
  Hope you will be resubmitting this.
 
 And here is the new version,
 I didn't make the name const as requested
 that field is being passed to the class_device_create
 method which requires a char* argument.
 
 But I have made the flag field.
 And now this time the patch actually includes the
 changes I promised in last mail.
 (mutex, workqueue api, etc)


Hi Ivo,

I am very sorry for taking so much time to respond but finally I went
through the patch and I still have the same objection as before -
it mixes two logically (and often physically) separated objects into
a single entity. I think that RF switch and button should be separate
entities, created and destroyed independently of each other. This way
everything handled uniformly by the kernel.

I attempted to rework the rfkill core supprt and simplify it and
came up with the patch below. Network card drivers that are able
to control state of their RF transmitters allocate and register
rfkill structures. Every rfkill structure belongs to one of
RF classes (WLAN, Bluetooth or IRDA) and exports its name, type,
state and user_claim through sysfs.

There is also an input handler that catces KEY_WLAN and KEY_BLUETOOTH
events passing through input system and toggles state of all RF
switches of appropriate type registered with rfkill system (unless
a switch was claimed by userspace in which case it is left alone).
I think this provides basic functionality that most of the users need
and any advanced control will have to be done from userspace.

In a followup patch there is a skeleton code for creating polled
input devices. For cards that have button physically connected
their drivers will have to register a separate input device and
let either input handler or userspace application take care of
switching RF state appropriately.

My only concern is where rfkill code should live. Since it is no
longer dependent on input core (embedded systems might disable
rfkill-input and use bare rfkill and control state from userspace)
it does not need to live in drivers/input.

Please let me know what you think.

-- 
Dmitry

From: Ivo van Doorn [EMAIL PROTECTED]

Input: rfkill - Add support for input key to control wireless radio

Signed-off-by Ivo van Doorn [EMAIL PROTECTED]
Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/input/misc/Kconfig|   26 ++
 drivers/input/misc/Makefile   |2 
 drivers/input/misc/rfkill-input.c |  169 
 drivers/input/misc/rfkill.c   |  400 ++
 include/linux/rfkill.h|   85 
 5 files changed, 682 insertions(+)

Index: work/drivers/input/misc/Kconfig
===
--- work.orig/drivers/input/misc/Kconfig
+++ work/drivers/input/misc/Kconfig
@@ -98,4 +98,30 @@ config HP_SDC_RTC
  Say Y here if you want to support the built-in real time clock
  of the HP SDC controller.
 
+config RFKILL
+   tristate RF switch support
+   help
+ Say Y here if you want to have control over RF switches
+ found on many WiFi, Bluetooth and IRDA cards.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rfkill.
+
+if RFKILL
+
+config RFKILL_INPUT
+   tristate Input layer to RF switch bridge
+   depends on INPUT
+   help
+ Say Y here if you want kernel automatically toggle state
+ of RF switches on and off when user presses appropriate
+ button an a key on the keyboard. Without this module
+ you need a some kind of userspace application to control
+ state of the switches.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rfkill-input.
+
+endif
+
 endif
Index: work/drivers/input/misc/Makefile
===
--- work.orig/drivers/input/misc/Makefile
+++ work/drivers/input/misc/Makefile
@@ -13,3 +13,5 @@ obj-$(CONFIG_INPUT_WISTRON_BTNS)  += wist
 obj-$(CONFIG_INPUT_ATLAS_BTNS) += atlas_btns.o
 obj-$(CONFIG_HP_SDC_RTC)   += hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IXP4XX_BEEPER)  += ixp4xx-beeper.o
+obj-$(CONFIG_RFKILL)   += rfkill.o
+obj-$(CONFIG_RFKILL_INPUT) += rfkill-input.o
Index: work/drivers/input/misc/rfkill.c
===
--- /dev/null
+++ work/drivers/input/misc/rfkill.c
@@ -0,0 +1,400 @@
+/*
+ * Copyright (C) 2006 Ivo van Doorn
+ * Copyright (C) 2007 Dmitry Torokhov
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On 2/8/07, Greg KH [EMAIL PROTECTED] wrote:


And it should also alow for proper power management functionality, using
the changes that Linus put into the driver core about 8 months ago.

Don't worry, I have input patches queued up next for you Dmitry :)



Greg,

Could you please forward me these patches for input core - I am going
to rearrange some code and don't want too many clashes so I'd like to
apply yours to my private three and work from there...

Thanks!

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On 2/9/07, Kay Sievers [EMAIL PROTECTED] wrote:

On Thu, 2007-02-08 at 22:59 -0500, Dmitry Torokhov wrote:
 On Thursday 08 February 2007 19:56, Greg KH wrote:
  On Thu, Feb 08, 2007 at 12:29:12PM -0500, Dmitry Torokhov wrote:
   On 2/8/07, Stephen Hemminger [EMAIL PROTECTED] wrote:
   On Thu, 08 Feb 2007 07:43:18 -0500

 Network: convert network devices to use struct device instead of
   class_device

 This lets the network core have the ability to handle
   suspend/resume
 issues, if it wants to.
   
   It fixes a non-problem. I would like to see the network core 
suspend/resume
   proposal as well. Last time I examined doing network core suspend help,
   the problem was that the physical device suspend was called before the
   class device. It is not clear how this change would help.
  
   If physical devices are registered before class devices then when
   suspending class devices are naturally suspended first. It is still
   not clear to me why we need to convert everythign to struct device, I
   believe I've shown (with patches) that it is possible to integrate
   struct class_device into PM framework and avoid reshuffling half of
   the kernel code.
 
  I don't want to have two separate device trees in the kernel (well, one
  big device tree and a bunch of little class_device trees.)  The code
  duplication in the class_device code is just too much, and I get
  questions all the time as to what the differences are.
 

 While duplication of code is a real concern my worry is constant fattening
 of struct device. For example most physical devices do not interface
 directly with userspace but every single one of them now has dev_t.
 Former class_devices do not need suspend/resume early framework either.
 And so on, and so forth.

The dev_t is a good example for the mess we try to fix here. Not having
a dev_t for devices lead to the creation of a lot of otherwise
completely useless class devices which are just a total pain to
interpret, and follow the events they create, from userspace.

Things like the scsi_device devices, usb_device devices, ... just exist,
because only this type of devices was allowed to pass information for
device nodes to userspace.



I admit I do not know scsi stack but I would expect that the only
things that need dev_t there would be sd, sr and sg interfaces. As
such they are separate entities and deserve their own structures no
mater what.

I can bet that number of real devices that need dev_t is smaller than
number of virtual devices that do not need full power management:

PCI cards, ACPI tree, etc, etc - hardware devices interfacing with
other parts of the kernel, not userspace directly.

NET, input, tty, etc - no need to suspend late/resume early

And, btw, having separate device and struct device does not prevent
exporting them as a unified sysfs tree and is in fact pretty easy to
do (I believe I posted patches to do that as well).

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On 2/8/07, Stephen Hemminger [EMAIL PROTECTED] wrote:

On Thu, 08 Feb 2007 07:43:18 -0500
Jeff Garzik [EMAIL PROTECTED] wrote:

 Linux Kernel Mailing List wrote:
  Gitweb: 
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
  Commit: 43cb76d91ee85f579a69d42bc8efc08bac560278
  Parent: 2943ecf2ed32632473c06f1975db47a7aa98c10f
  Author: Greg Kroah-Hartman [EMAIL PROTECTED]
  AuthorDate: Tue Apr 9 12:14:34 2002 -0700
  Committer:  Greg Kroah-Hartman [EMAIL PROTECTED]
  CommitDate: Wed Feb 7 10:37:11 2007 -0800
 
  Network: convert network devices to use struct device instead of 
class_device
 
  This lets the network core have the ability to handle suspend/resume
  issues, if it wants to.

It fixes a non-problem. I would like to see the network core suspend/resume
proposal as well. Last time I examined doing network core suspend help,
the problem was that the physical device suspend was called before the
class device. It is not clear how this change would help.


If physical devices are registered before class devices then when
suspending class devices are naturally suspended first. It is still
not clear to me why we need to convert everythign to struct device, I
believe I've shown (with patches) that it is possible to integrate
struct class_device into PM framework and avoid reshuffling half of
the kernel code.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On Thursday 08 February 2007 19:56, Greg KH wrote:
 On Thu, Feb 08, 2007 at 12:29:12PM -0500, Dmitry Torokhov wrote:
  On 2/8/07, Stephen Hemminger [EMAIL PROTECTED] wrote:
  On Thu, 08 Feb 2007 07:43:18 -0500
  Jeff Garzik [EMAIL PROTECTED] wrote:
  
   Linux Kernel Mailing List wrote:
Gitweb: 
  http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
Commit: 43cb76d91ee85f579a69d42bc8efc08bac560278
Parent: 2943ecf2ed32632473c06f1975db47a7aa98c10f
Author: Greg Kroah-Hartman [EMAIL PROTECTED]
AuthorDate: Tue Apr 9 12:14:34 2002 -0700
Committer:  Greg Kroah-Hartman [EMAIL PROTECTED]
CommitDate: Wed Feb 7 10:37:11 2007 -0800
   
Network: convert network devices to use struct device instead of 
  class_device
   
This lets the network core have the ability to handle 
  suspend/resume
issues, if it wants to.
  
  It fixes a non-problem. I would like to see the network core suspend/resume
  proposal as well. Last time I examined doing network core suspend help,
  the problem was that the physical device suspend was called before the
  class device. It is not clear how this change would help.
  
  If physical devices are registered before class devices then when
  suspending class devices are naturally suspended first. It is still
  not clear to me why we need to convert everythign to struct device, I
  believe I've shown (with patches) that it is possible to integrate
  struct class_device into PM framework and avoid reshuffling half of
  the kernel code.
 
 I don't want to have two separate device trees in the kernel (well, one
 big device tree and a bunch of little class_device trees.)  The code
 duplication in the class_device code is just too much, and I get
 questions all the time as to what the differences are.


While duplication of code is a real concern my worry is constant fattening
of struct device. For example most physical devices do not interface
directly with userspace but every single one of them now has dev_t.
Former class_devices do not need suspend/resume early framework either.
And so on, and so forth.
 
 With these slow and gradual changes, we are getting a true, unified,
 device tree, and it will reduce the amount of code and complexity we
 need to maintain and fix in the driver core itself.
 
 And it should also alow for proper power management functionality, using
 the changes that Linus put into the driver core about 8 months ago.
 
 Don't worry, I have input patches queued up next for you Dmitry :)
 

;)

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

Hi Ivo,

On Thursday 07 December 2006 16:53, Ivo van Doorn wrote:
 Hi,
 
  2 - Hardware key that does not control the hardware radio and does 
 not report anything to userspace
   
Kind of uninteresting button ;)
  
   And this is the button that rfkill was originally designed for.
   Laptops with integrated WiFi cards from Ralink have a hardware button 
   that don't send anything to
   userspace (unless the ACPI event is read) and does not directly control 
   the radio itself.
  
  
  So what does such a button do? I am confused here...
 
 Without a handler like rfkill, it does nothing besides toggling a bit in a 
 register.
 The Ralink chipsets have a couple of registers that represent the state of 
 that key.
 Besides that, there are no notifications to the userspace nor does it 
 directly control the
 radio.
 That is where rfkill came in with the toggle handler that will listen to the 
 register
 to check if the key has been pressed and properly process the key event.

In this case the driver can make the button state available to userspace so
thsi is really type 2) driver as far as I can see. The fact that the button
is not reported to userspace yet should not get into our way of classifying
it.

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH wireless-2.6-git] prism54: WPA/RSN support for fullmac cards

[Dmitry Torokhov]

On 12/6/06, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote:

On Wednesday, 6. December 2006 02:44, you wrote:
 FYI...after latest round of merges, that patch that I've been carrying
 in the pending branch of wireless-2.6 no longer applies.

 John

Ok, I'm waiting for Dmitry' feedback...  (hello, where're you ;) ?)



Sorry, busy time... I still have to do ifup eth1 manually after
inserting the card to get it going.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 12/4/06, Ivo van Doorn [EMAIL PROTECTED] wrote:

 I am still not sure that tight coupling of input device with rfkill
 structure is such a good idea. Quite often the button is separated
 from the device itself and radio control is done via BIOS SMM (see
 wistron driver) or there is no special button at all and users might
 want to assign one of their standard keyboard buttons to be an RF
 switch.

Making sure rfkill supports keys that are not handled by the driver
is a bit hard. Just as drivers that can only check if the button is
toggled and not what the current state is.
The problem is that it is hard to make a clean split between the
2 different button controls. Not all drivers allow the radio to be
enabled while the button status are indicating the radio should
be off.


If they do not allow controlling the state of the radio
programmatically then it should not be part of rfkill I am afraid. It
is like the power switch - if you hold it for so long it kills the
power to the box and there is nothing you can do about it.


The buttons that are already integrated into the keyboard,
by example by using a Fn key combo don't control the device
directly. So the driver cannot offer anything to the rfkill driver.
Such buttons should be mapped in userspace without the help of rfkill,
since the kernel cannot detect if that key belonged to a radio
control key or not.



That is my point. Given the fact that there are keys that are not
directly connected with the radio switch userspace will have to handle
them (wait for events then turn off radios somehow). You are
advocating that userspace should also implement 2nd method for buttons
that belong to rfkill interface. I do not understand the need for 2nd
interface. If you separate radio switch from button code then
userspace only need to implement 1st interface and be done with it.
You will have set of cards that provide interface to enable/disable
their transmitters and set of buttons that signal userspace desired
state change. If both switch and button is implemented by the same
driver then the driver can implement automatic button handling.
Otherwise userspace help is necessary.


 I think it would be better if there was an rfkill class listing all
 controlled devices (preferrably grouped by their type - WiFi, BT,
 IRDA, etc) and if every group would provide an attribute allowing to
 control state of the whole group (do we realistically need to kill
 just one interface? Wouldn't ifconfig be suitable for that?). The

There have been mixed feelings on the netdev list about what should
exactly happen when the button is pressed. The possible options are:

1 - rfkill will kill all interfaces
2 - rfkill will kill all interfaces of the same type (wifi, bt, irda)
3 - rfkill will kill the interface it belongs to

Personally I would favour the second option, but used the third after hearing
objections to the second method. So since there are also fans of
the third option I think there should be a decision made about what the
correct option is, so rfkill can follow that method.


Fans of the 3rd method, speak up ;)



 attribute should be a tri-state on/off/auto, auto meaning the driver
 itself manages radio state. This would avoid another tacky IMHO point
 that in your implementation mere opening of an input device takes over
 RF driver. Explicit control allow applications snoop RF state
 without disturbing it.

Currently userspace can always check the state of the button whenever
they like by checking the sysfs entry.



Unless the key is not directly connected to the driver (so there is no
sysfs entry). Again you force 2 different interfaces.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 12/6/06, Dan Williams [EMAIL PROTECTED] wrote:

On Wed, 2006-12-06 at 09:37 -0500, Dmitry Torokhov wrote:

 Fans of the 3rd method, speak up ;)

I think I brought up the 3rd method initially in this thread.  I'm not
necessarily advocating it, but I wanted to be sure people realized that
this was a case, so that a clear decision would be made to support it or
not to support it.

(2) makes the most sense to me.  I don't think we need to care about
edge-cases like But I only wanted to rfkill _one_ of my bluetooth
dongles!!!, that's just insane.

But using (2) also begs the question, can we _always_ identify what
interface the rfkill belongs to?  In Bastien's laptop, the rfkill switch
_automatically_ disconnects the internal USB Bluetooth device from the
USB bus, and uses the normal ipw2200 rfkill mechanism, whatever that is.
In this case, you simply do not get an event that the bluetooth device
is disabled from a button somewhere; it's just gone, and you'd have to
do some magic to disable other bluetooth devices as well.



Is this the same physical button? If so then for this particular box
we'd just have to send 2 events - KEY_WIFI and KEY_BLUETOOTH at the
same time.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 12/6/06, Ivo van Doorn [EMAIL PROTECTED] wrote:

On Wednesday 06 December 2006 15:37, Dmitry Torokhov wrote:
 On 12/4/06, Ivo van Doorn [EMAIL PROTECTED] wrote:
   I am still not sure that tight coupling of input device with rfkill
   structure is such a good idea. Quite often the button is separated
   from the device itself and radio control is done via BIOS SMM (see
   wistron driver) or there is no special button at all and users might
   want to assign one of their standard keyboard buttons to be an RF
   switch.
 
  Making sure rfkill supports keys that are not handled by the driver
  is a bit hard. Just as drivers that can only check if the button is
  toggled and not what the current state is.
  The problem is that it is hard to make a clean split between the
  2 different button controls. Not all drivers allow the radio to be
  enabled while the button status are indicating the radio should
  be off.

 If they do not allow controlling the state of the radio
 programmatically then it should not be part of rfkill I am afraid. It
 is like the power switch - if you hold it for so long it kills the
 power to the box and there is nothing you can do about it.

Ok, this will give rfkill more possibilities as I could in that case
also allow the user to toggle the radio to the state that is different
than indicated by the key.
Currently this was not possible since I had to keep in mind that there
were keys that would directly control the radio.

  The buttons that are already integrated into the keyboard,
  by example by using a Fn key combo don't control the device
  directly. So the driver cannot offer anything to the rfkill driver.
  Such buttons should be mapped in userspace without the help of rfkill,
  since the kernel cannot detect if that key belonged to a radio
  control key or not.
 

 That is my point. Given the fact that there are keys that are not
 directly connected with the radio switch userspace will have to handle
 them (wait for events then turn off radios somehow). You are
 advocating that userspace should also implement 2nd method for buttons
 that belong to rfkill interface. I do not understand the need for 2nd
 interface. If you separate radio switch from button code then
 userspace only need to implement 1st interface and be done with it.
 You will have set of cards that provide interface to enable/disable
 their transmitters and set of buttons that signal userspace desired
 state change. If both switch and button is implemented by the same
 driver then the driver can implement automatic button handling.
 Otherwise userspace help is necessary.

Well there are 3 possible hardware key approaches:

 1 - Hardware key that controls the hardware radio, and does not report 
anything to userspace


Can't do anything here so just ignore it.


 2 - Hardware key that does not control the hardware radio and does not report 
anything to userspace


Kind of uninteresting button ;)


 3 - Hardware key that does not control the hardware radio and reports the key 
to userspace

So rfkill should not be used in the case of (1) and (3), but we still need 
something to support (2)
or should the keys not be handled by userspace and always by the driver?
This is making rfkill moving slowly away from the generic approach for all 
rfkill keys as the initial
intention was.



I my vision rfkill would represent userspace namageable radio
switch. We have the followng possible configurations:

1. A device that does not allow controlling its transmitter from
userspace. The driver should not use/register with rfkill subsystem as
userspace can't do anyhting with it. If device has a button killing
the transmitter the driver can still signal userspace appropriate
event (KEY_WIFI, KEY_BLUETOOTH, etc) if it can detect that button was
presssed so userspace can monitor state of the transmitter and
probably shut down other transmitters to keep everything in sync.

2. A device that does allow controlling its transmitter. The driver
may (should) register with rfkill subsystem. Additionally, if there is
a button, the driver should register it with input subsystem. Driver
should manage transmitter state in response to button presses unless
userspace takes over the process.

3. A device without transmitter but with a button - just register with
input core. Userspace will have to manage state of other devices with
transmitters in response to button presses.

Does this make sense?


   attribute should be a tri-state on/off/auto, auto meaning the driver
   itself manages radio state. This would avoid another tacky IMHO point
   that in your implementation mere opening of an input device takes over
   RF driver. Explicit control allow applications snoop RF state
   without disturbing it.
 
  Currently userspace can always check the state of the button whenever
  they like by checking the sysfs entry.
 

 Unless the key is not directly connected to the driver (so there is no
 sysfs entry). Again you force 2 different

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 12/6/06, Ivo van Doorn [EMAIL PROTECTED] wrote:


   2 - Hardware key that does not control the hardware radio and does not 
report anything to userspace

 Kind of uninteresting button ;)

And this is the button that rfkill was originally designed for.
Laptops with integrated WiFi cards from Ralink have a hardware button that 
don't send anything to
userspace (unless the ACPI event is read) and does not directly control the 
radio itself.



So what does such a button do? I am confused here...

...


And this event should be reported by a generic approach right? So it should
be similar as with your point 2 below. But this would mean that the driver
should create the input device. Or can a driver send the KEY_WIFI event
over a main layer without the need of a personal input device?
I am not that familiar with the input device layer in the kernel, and this is
my first attempt on creating something for it, so I might have missed 
something. ;)


Yes, I think the driver should just create an input device. You may
provide a generic implementation for a polled button and have driver
instantiate it but I do not think that a single RFkill button device
is needed - you won't have too many of them in a single system anyway
(I think you will normally have 1, 2 at the most).

...

 3. A device without transmitter but with a button - just register with
 input core. Userspace will have to manage state of other devices with
 transmitters in response to button presses.

This is clear too. Rfkill is only intended for drivers that control a device 
with
a transmitter (WiFi, Bluetooth, IRDA) that have a button that is intended to
do something with the radio/transmitter.

 Does this make sense?

Yes, this was what I intended to do with rfkill, so at that point we have
the same goal.



I think it is almost the same. I also want support RF devices that can
control radio state but lack a button. This is covered by mixing 2)
and 3) in kernel and for userspace looks exactly like 2) with a
button.

...


 I don't think a config option is a good idea unless by config option
 you mean a sysfs attribute.

I indeed meant a sysfs attribute. I should have been more clear on this. :)



OK :)

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 12/6/06, Jiri Benc [EMAIL PROTECTED] wrote:

On Wed, 6 Dec 2006 15:18:12 -0500, Dmitry Torokhov wrote:
 On 12/6/06, Ivo van Doorn [EMAIL PROTECTED] wrote:
  Ok, so input device opening should not block the rfkill signal and the 
rfkill handler
  should still go through with its work unless a different config option 
indicates that
  userspace wants to handle the event.

 I don't think a config option is a good idea unless by config option
 you mean a sysfs attribute.

What about using EVIOCGRAB ioctl for this?



Will not work when the button is on your USB keyboard ;)

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] rfkill - Add support for input key to control wireless radio

[Dmitry Torokhov]

On 12/3/06, Ivo van Doorn [EMAIL PROTECTED] wrote:

Hi,

This patch is a resend of a patch I has been send to the linux kernel
and netdev list earlier. The most recent version of a few weeks back
didn't compile since I missed 1 line in my patch that changed
include/linux/input.h.

This patch will offer the handling of radiokeys on a laptop.
Such keys often control the wireless devices on the radio
like the wifi, bluetooth and irda.

The rfkill works as follows, when the user presses the hardware key
to control the wireless (wifi, bluetooth or irda) radio a signal will
go to rfkill. This happens by the hardware driver sending a signal
to rfkill, or rfkill polling for the button status.
The key signal will then be processed by rfkill, each key will have
its own input device, if this input device has not been openened
by the user, rfkill will keep the signal and either turn the radio
on or off based on the key status.
If the input device has been opened, rfkill will send the signal to
userspace and do nothing further. The user is in charge of controlling
the radio.

This driver (if accepted and applied) will be usefull for the rt2x00 drivers
(rt2400pci, rt2500pci, rt2500usb, rt61pci and rt73usb) in the wireless-dev
tree and the MSI laptop driver from Lennart Poettering in the main
linux kernel tree.

Before this patch can be applied to any tree, I first wish to hear
if the patch is acceptable. Since the previous time it was send I have made
several fixes based on the feedback like adding the sysfs entries for
reading the status.



Hi Ivo,

I apologize for not responding to your post earlier, it was a busy week.

I am still not sure that tight coupling of input device with rfkill
structure is such a good idea. Quite often the button is separated
from the device itself and radio control is done via BIOS SMM (see
wistron driver) or there is no special button at all and users might
want to assign one of their standard keyboard buttons to be an RF
switch.

I think it would be better if there was an rfkill class listing all
controlled devices (preferrably grouped by their type - WiFi, BT,
IRDA, etc) and if every group would provide an attribute allowing to
control state of the whole group (do we realistically need to kill
just one interface? Wouldn't ifconfig be suitable for that?). The
attribute should be a tri-state on/off/auto, auto meaning the driver
itself manages radio state. This would avoid another tacky IMHO point
that in your implementation mere opening of an input device takes over
RF driver. Explicit control allow applications snoop RF state
without disturbing it.

If there are concerns that drivers will have to re-implement most of
the button handling you are still free to create a simple
implementation of polled RF button (I don't think that interrupt
driver RF buttons would share alot of code) so that users would only
need to implement a polling function.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH wireless-2.6-git] prism54: WPA/RSN support for fullmac cards

[Dmitry Torokhov]

On 11/17/06, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote:

Am Freitag, 17. November 2006 06:42 schrieben Sie:
 
  No, more that my card doesn't successfully associate at all even
  _without_ the patch on unencrypted or WEP APs; I just can't test it,
  since my card seems to be broken...

 FWIW I applied the patch to tonight pull form Linus's tree and my card
 stopped associating with my AP (WEP128, Shared key). It only associated
 with my neighbour router which is running without encryption.
can you please describe your setup? I tested WEP (restricted) with a Netgear
WGT624 and a normal linux AP and haven't seen any problems so far.



The card is WG511 talking to netgear WGR614 router. I am using WEP128
with shared keys. Router is set to be g only. The OS is Fedora Core
3 (I swear I will upgrade :) ), just recompiled wireless tools from
sources in FC6.


for debugging:
dmesg  iwevent output would be nice...


Will do when I get home.


but I'll accept a short sniffer extract too. ;)

(it would be really good, if you can provide a package dump form a working
setup with the old driver too!)



You'd have to tell me exactly what you want me to do.

The reason for me trying the patch is that the card locks up
frequently (the amber transmission light turns on stays this way).

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH wireless-2.6-git] prism54: WPA/RSN support for fullmac cards

[Dmitry Torokhov]

On 11/17/06, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote:


 The reason for me trying the patch is that the card locks up
 frequently (the amber transmission light turns on stays this way).

Do you get any fancy mgt timeout ... or something in dmesg, when
the cards locks up? (include one, if you can, maybe syslog saved one)



I think I saw these before (with earlier kernels, can't tell you
exactly when it stopped as I don't pay much attention to versions - I
work off tip of Linus's tree and pull often) but now it looks like it
simply locks up. Maybe I don't wait long enough - I just simply eject
and inster the card back when I see it locked up.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH wireless-2.6-git] prism54: WPA/RSN support for fullmac cards

[Dmitry Torokhov]

On 11/17/06, Dmitry Torokhov [EMAIL PROTECTED] wrote:

On 11/17/06, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote:
 
  The reason for me trying the patch is that the card locks up
  frequently (the amber transmission light turns on stays this way).

 Do you get any fancy mgt timeout ... or something in dmesg, when
 the cards locks up? (include one, if you can, maybe syslog saved one)


I think I saw these before (with earlier kernels, can't tell you
exactly when it stopped as I don't pay much attention to versions - I
work off tip of Linus's tree and pull often) but now it looks like it
simply locks up. Maybe I don't wait long enough - I just simply eject
and inster the card back when I see it locked up.



Btw, it usually locks up under heavyish load. I.e. if I just ssh into
my devel box and code in vi it will stay jsut fine for hours. But
FTPing stuff over quite often locks it up.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH wireless-2.6-git] prism54: WPA/RSN support for fullmac cards

[Dmitry Torokhov]

On Friday 17 November 2006 15:53, [EMAIL PROTECTED] wrote:
 
  The card is WG511 talking to netgear WGR614 router. I am using WEP128
  with shared keys. Router is set to be g only. The OS is Fedora Core
  3 (I swear I will upgrade :) ), just recompiled wireless tools from
  sources in FC6.
 
   for debugging:
   dmesg  iwevent output would be nice...
 
  Will do when I get home.
 
   but I'll accept a short sniffer extract too. ;)
  
   (it would be really good, if you can provide a package dump form a
   working setup with the old driver too!)
 
  You'd have to tell me exactly what you want me to do.
 just the Management Frames (MLME):
 Beacon, Authentication and Association Responses/Requests Frames
 you can capture these frames with an extra sniffer card (in monitor mode)
 just run tcpdump, wireshark(ethereal), kismet, ...


OK, i'll try installing kismet on another laptop. For now that's what I have
from iwevent:

[EMAIL PROTECTED] ~]# cat iwevents.bad
21:30:52.185823   eth1 Set Mode:Managed
21:30:52.226100   eth1 Set ESSID:ALCT76_68b
21:30:54.505039   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:30:56.300128   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:30:58.095251   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:11.413225   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:13.207322   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:15.003497   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:16.798687   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:18.593844   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:31.911710   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:33.705200   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:35.501650   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:37.296756   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:39.091866   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:52.409596   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]
21:31:54.203281   eth1 Custom driver event:Authenticate request to 
00:0f:b5:5b:fe:3a [00 0D 0002]

[EMAIL PROTECTED] ~]# cat iwevents.good
21:50:50.575630   eth1 Set Mode:Managed
21:50:50.604998   eth1 Set Encryption key:------**  
 Security mode:restricted
21:50:50.616502   eth1 Set ESSID:ALCT76_68b
21:50:52.880361   eth1 Custom driver event:Authenticate request to 
00:0F:B5:5B:FE:3A  : ACCEPTED  (00)
21:50:52.882638   eth1 Custom driver event:Associate request to 
00:0F:B5:5B:FE:3A  : ACCEPTED  (00)
21:50:52.883625   eth1 New Access Point/Cell address:00:0F:B5:5B:FE:3A

 
 
 
  The reason for me trying the patch is that the card locks up
  frequently (the amber transmission light turns on stays this way).
  
 Do you get any fancy mgt timeout ... or something in dmesg, when
 the cards locks up? (include one, if you can, maybe syslog saved one)
 

No, just netdev watchdog bitches:

Nov 16 23:15:30 core kernel: [164606.712000] NETDEV WATCHDOG: eth1: transmit 
timed out
Nov 16 23:15:30 core kernel: [164606.712000] eth1: tx_timeout, scheduling a 
reset
Nov 16 23:15:30 core kernel: [164606.712000] eth1: resetting device...
Nov 16 23:15:30 core kernel: [164606.712000] eth1: uploading firmware...
Nov 16 23:15:31 core kernel: [164606.996000] eth1: firmware version: 1.0.4.3
Nov 16 23:15:31 core kernel: [164606.996000] eth1: firmware upload complete
Nov 16 23:15:31 core kernel: [164607.232000] eth1: interface reset complete

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH wireless-2.6-git] prism54: WPA/RSN support for fullmac cards

[Dmitry Torokhov]

On Friday 10 November 2006 09:42, Dan Williams wrote:
 On Thu, 2006-11-09 at 18:16 -0500, Luis R. Rodriguez wrote:
  On 11/9/06, Luis R. Rodriguez [EMAIL PROTECTED] wrote:
   On 11/9/06, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote:
Am Mittwoch, 8. November 2006 01:39 schrieben Sie:
 On Fri, Nov 03, 2006 at 01:41:46PM -0500, Luis R. Rodriguez wrote:
  On 11/3/06, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote:
  yes, especially mgt_commit_list caused alot headaches, until I 
  removed
  DOT11_OID_PSM from the cache list.
  Now, I can hammer it with ping -f for hours.
 
  nice, perhaps that's been the culprit all along... going to dig to 
  see
  if I find a fullmac prism card. Will like to get this merged in.

 Any resolution on this?
   
no replies.
Seems like it works for just fine for everybody. ;)
  
   I found a card, I just need time to test it. Dan didn't you say you
   ran into issues with the patch on your card?
 
 No, more that my card doesn't successfully associate at all even
 _without_ the patch on unencrypted or WEP APs; I just can't test it,
 since my card seems to be broken...


FWIW I applied the patch to tonight pull form Linus's tree and my card
stopped associating with my AP (WEP128, Shared key). It only associated
with my neighbour router which is running without encryption.

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 2/3] atmel: use ARRAY_SIZE()

[Dmitry Torokhov]

NET: atmel - switch to using ARRAY_SIZE()

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/net/wireless/atmel.c |   20 ++--
 1 files changed, 10 insertions(+), 10 deletions(-)

Index: work/drivers/net/wireless/atmel.c
===
--- work.orig/drivers/net/wireless/atmel.c
+++ work/drivers/net/wireless/atmel.c
@@ -1193,7 +1193,7 @@ static irqreturn_t service_interrupt(int
 
atmel_set_gcr(dev, GCR_ACKINT); /* acknowledge interrupt */
 
-   for (i = 0; i  sizeof(irq_order)/sizeof(u8); i++)
+   for (i = 0; i  ARRAY_SIZE(irq_order); i++)
if (isr  irq_order[i])
break;
 
@@ -1345,10 +1345,10 @@ int atmel_open(struct net_device *dev)
atmel_set_mib8(priv, Phy_Mib_Type, PHY_MIB_REG_DOMAIN_POS, 
priv-reg_domain);
} else {
priv-reg_domain = atmel_get_mib8(priv, Phy_Mib_Type, 
PHY_MIB_REG_DOMAIN_POS);
-   for (i = 0; i  sizeof(channel_table)/sizeof(channel_table[0]); 
i++)
+   for (i = 0; i  ARRAY_SIZE(channel_table); i++)
if (priv-reg_domain == channel_table[i].reg_domain)
break;
-   if (i == sizeof(channel_table)/sizeof(channel_table[0])) {
+   if (i == ARRAY_SIZE(channel_table)) {
priv-reg_domain = REG_DOMAIN_MKK1;
printk(KERN_ALERT %s: failed to get regulatory domain: 
assuming MKK1.\n, dev-name);
}
@@ -1393,7 +1393,7 @@ static int atmel_validate_channel(struct
   else return suitable default channel */
int i;
 
-   for (i = 0; i  sizeof(channel_table)/sizeof(channel_table[0]); i++)
+   for (i = 0; i  ARRAY_SIZE(channel_table); i++)
if (priv-reg_domain == channel_table[i].reg_domain) {
if (channel = channel_table[i].min 
channel = channel_table[i].max)
@@ -1437,7 +1437,7 @@ static int atmel_proc_output (char *buf,
}
 
r = unknown;
-   for (i = 0; i  sizeof(channel_table)/sizeof(channel_table[0]); 
i++)
+   for (i = 0; i  ARRAY_SIZE(channel_table); i++)
if (priv-reg_domain == channel_table[i].reg_domain)
r = channel_table[i].name;
 
@@ -2375,7 +2375,7 @@ static int atmel_get_range(struct net_de
range-min_nwid = 0x;
range-max_nwid = 0x;
range-num_channels = 0;
-   for (j = 0; j  sizeof(channel_table)/sizeof(channel_table[0]); j++)
+   for (j = 0; j  ARRAY_SIZE(channel_table); j++)
if (priv-reg_domain == channel_table[j].reg_domain) {
range-num_channels = channel_table[j].max - 
channel_table[j].min + 1;
break;
@@ -2581,9 +2581,9 @@ static const struct iw_priv_args atmel_p
 
 static const struct iw_handler_def atmel_handler_def =
 {
-   .num_standard   = sizeof(atmel_handler)/sizeof(iw_handler),
-   .num_private= sizeof(atmel_private_handler)/sizeof(iw_handler),
-   .num_private_args = sizeof(atmel_private_args)/sizeof(struct 
iw_priv_args),
+   .num_standard   = ARRAY_SIZE(atmel_handler),
+   .num_private= ARRAY_SIZE(atmel_private_handler),
+   .num_private_args = ARRAY_SIZE(atmel_private_args),
.standard   = (iw_handler *) atmel_handler,
.private= (iw_handler *) atmel_private_handler,
.private_args   = (struct iw_priv_args *) atmel_private_args,
@@ -2647,7 +2647,7 @@ static int atmel_ioctl(struct net_device
 
domain[REGDOMAINSZ] = 0;
rc = -EINVAL;
-   for (i = 0; i  sizeof(channel_table)/sizeof(channel_table[0]); 
i++) {
+   for (i = 0; i  ARRAY_SIZE(channel_table); i++) {
/* strcasecmp doesn't exist in the library */
char *a = channel_table[i].name;
char *b = domain;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 3/3] atmel: whitespace cleanup

[Dmitry Torokhov]

NET: atmel: whitespace cleanup

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/net/wireless/atmel.c |6 +--
 drivers/net/wireless/atmel_cs.c  |   74 +++
 drivers/net/wireless/atmel_pci.c |   10 ++---
 3 files changed, 45 insertions(+), 45 deletions(-)

Index: work/drivers/net/wireless/atmel.c
===
--- work.orig/drivers/net/wireless/atmel.c
+++ work/drivers/net/wireless/atmel.c
@@ -595,7 +595,7 @@ static void atmel_join_bss(struct atmel_
 static void atmel_smooth_qual(struct atmel_private *priv);
 static void atmel_writeAR(struct net_device *dev, u16 data);
 static int probe_atmel_card(struct net_device *dev);
-static int reset_atmel_card(struct net_device *dev );
+static int reset_atmel_card(struct net_device *dev);
 static void atmel_enter_state(struct atmel_private *priv, int new_state);
 int atmel_open (struct net_device *dev);
 
@@ -1738,7 +1738,7 @@ static int atmel_set_encode(struct net_d
/* Disable the key */
priv-wep_key_len[index] = 0;
/* Check if the key is not marked as invalid */
-   if(!(dwrq-flags  IW_ENCODE_NOKEY)) {
+   if (!(dwrq-flags  IW_ENCODE_NOKEY)) {
/* Cleanup */
memset(priv-wep_keys[index], 0, 13);
/* Copy the key in the driver */
@@ -1909,7 +1909,7 @@ static int atmel_get_encodeext(struct ne
 
encoding-flags = idx + 1;
memset(ext, 0, sizeof(*ext));
-   
+
if (!priv-wep_is_on) {
ext-alg = IW_ENCODE_ALG_NONE;
ext-key_len = 0;
Index: work/drivers/net/wireless/atmel_cs.c
===
--- work.orig/drivers/net/wireless/atmel_cs.c
+++ work/drivers/net/wireless/atmel_cs.c
@@ -5,12 +5,12 @@
 Copyright 2000-2001 ATMEL Corporation.
 Copyright 2003 Simon Kelley.
 
-This code was developed from version 2.1.1 of the Atmel drivers, 
-released by Atmel corp. under the GPL in December 2002. It also 
-includes code from the Linux aironet drivers (C) Benjamin Reed, 
-and the Linux PCMCIA package, (C) David Hinds. 
+This code was developed from version 2.1.1 of the Atmel drivers,
+released by Atmel corp. under the GPL in December 2002. It also
+includes code from the Linux aironet drivers (C) Benjamin Reed,
+and the Linux PCMCIA package, (C) David Hinds.
 
-For all queries about this code, please contact the current author, 
+For all queries about this code, please contact the current author,
 Simon Kelley [EMAIL PROTECTED] and not Atmel Corporation.
 
 This program is free software; you can redistribute it and/or modify
@@ -87,7 +87,7 @@ MODULE_SUPPORTED_DEVICE(Atmel at76c50x 
event is received.  The config() and release() entry points are
used to configure or release a socket, in response to card
insertion and ejection events.  They are invoked from the atmel_cs
-   event handler. 
+   event handler.
 */
 
 static int atmel_config(struct pcmcia_device *link);
@@ -133,22 +133,22 @@ static void atmel_detach(struct pcmcia_d
device IO routines can use a flag like this to throttle IO to a
card that is not ready to accept it.
 */
-   
+
 typedef struct local_info_t {
dev_node_t  node;
struct net_device *eth_dev;
 } local_info_t;
 
 /*==
-  
+
   atmel_attach() creates an instance of the driver, allocating
   local data structures for one device.  The device is registered
   with Card Services.
-  
+
   The dev_link structure is initialized, but we don't actually
   configure the card at this point -- we wait until we receive a
   card insertion event.
-  
+
   ==*/
 
 static int atmel_probe(struct pcmcia_device *p_dev)
@@ -184,12 +184,12 @@ static int atmel_probe(struct pcmcia_dev
 } /* atmel_attach */
 
 /*==
-  
+
   This deletes a driver instance.  The device is de-registered
   with Card Services.  If it has been released, all local data
   structures are freed.  Otherwise, the structures will be freed
   when the device is released.
-  
+
   ==*/
 
 static void atmel_detach(struct pcmcia_device *link)
@@ -202,11 +202,11 @@ static void atmel_detach(struct pcmcia_d
 }
 
 /*==
-  
+
   atmel_config() is scheduled to run after a CARD_INSERTION event
   is received, to configure the PCMCIA socket, and to make the
   device available to the system.
-  
+
   ==*/
 
 #define CS_CHECK(fn, ret

[patch 0/3] atmel: couple of cleanup patches

[Dmitry Torokhov]

Hi,

Here are couple of cleanup patches for atmel driver. They are
against tonight pull from Linus. Please consider applying.

Thanks!

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 1/3] atmel: save on array initialization

[Dmitry Torokhov]

NET: atmel - do not initialize array over and over again

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/net/wireless/atmel.c |2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: work/drivers/net/wireless/atmel.c
===
--- work.orig/drivers/net/wireless/atmel.c
+++ work/drivers/net/wireless/atmel.c
@@ -784,11 +784,11 @@ static void tx_update_descriptor(struct 
 
 static int start_tx(struct sk_buff *skb, struct net_device *dev)
 {
+   static const u8 SNAP_RFC1024[6] = { 0xaa, 0xaa, 0x03, 0x00, 0x00, 0x00 
};
struct atmel_private *priv = netdev_priv(dev);
struct ieee80211_hdr_4addr header;
unsigned long flags;
u16 buff, frame_ctl, len = (ETH_ZLEN  skb-len) ? skb-len : ETH_ZLEN;
-   u8 SNAP_RFC1024[6] = {0xaa, 0xaa, 0x03, 0x00, 0x00, 0x00};
 
if (priv-card  priv-present_callback 
!(*priv-present_callback)(priv-card)) {
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 0/2] prism54: couple of cleanup patches

[Dmitry Torokhov]

Hi,

Here are couple of patches for prism54 driver, they are against
tonight pull from Linus. Please consider applying.

Thanks!

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 1/2] prism54: fix potential race in reset scheduling

[Dmitry Torokhov]

NET: prism54 - fix potential race in reset scheduling

There appears to be a race in reset scheduling logic - thread
responsible for reseting the interface should clear reset
pending flag before restarting the queue, otherwise timeout
handler might not schedule another reset even if it is needed.

This race is mostly theoretical as far as I can see but a race
nonetheless.

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/net/wireless/prism54/islpci_eth.c |   23 +--
 1 files changed, 13 insertions(+), 10 deletions(-)

Index: work/drivers/net/wireless/prism54/islpci_eth.c
===
--- work.orig/drivers/net/wireless/prism54/islpci_eth.c
+++ work/drivers/net/wireless/prism54/islpci_eth.c
@@ -253,6 +253,7 @@ islpci_monitor_rx(islpci_private *priv, 
 * header and without the FCS. But there a is a bit that
 * indicates if the packet is corrupted :-) */
struct rfmon_header *hdr = (struct rfmon_header *) (*skb)-data;
+
if (hdr-flags  0x01)
/* This one is bad. Drop it ! */
return -1;
@@ -464,10 +465,8 @@ islpci_eth_receive(islpci_private *priv)
break;
}
/* update the fragment address */
-   control_block-rx_data_low[index].address = cpu_to_le32((u32)
-   priv-
-   
pci_map_rx_address
-   
[index]);
+   control_block-rx_data_low[index].address =
+   cpu_to_le32((u32)priv-pci_map_rx_address[index]);
wmb();
 
/* increment the driver read pointer */
@@ -484,10 +483,12 @@ islpci_eth_receive(islpci_private *priv)
 void
 islpci_do_reset_and_wake(void *data)
 {
-   islpci_private *priv = (islpci_private *) data;
+   islpci_private *priv = data;
+
islpci_reset(priv, 1);
-   netif_wake_queue(priv-ndev);
priv-reset_task_pending = 0;
+   smp_wmb();
+   netif_wake_queue(priv-ndev);
 }
 
 void
@@ -499,12 +500,14 @@ islpci_eth_tx_timeout(struct net_device 
/* increment the transmit error counter */
statistics-tx_errors++;
 
-   printk(KERN_WARNING %s: tx_timeout, ndev-name);
if (!priv-reset_task_pending) {
-   priv-reset_task_pending = 1;
-   printk(, scheduling a reset);
+   printk(KERN_WARNING
+   %s: tx_timeout, scheduling reset, ndev-name);
netif_stop_queue(ndev);
+   priv-reset_task_pending = 1;
schedule_work(priv-reset_task);
+   } else {
+   printk(KERN_WARNING
+   %s: tx_timeout, waiting for reset, ndev-name);
}
-   printk(\n);
 }
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 2/2] prism54: whitespace cleanup

[Dmitry Torokhov]

NET: prism54 - whitespace cleanup

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/net/wireless/prism54/isl_38xx.c   |   17 +++
 drivers/net/wireless/prism54/isl_38xx.h   |7 +-
 drivers/net/wireless/prism54/isl_ioctl.c  |   61 --
 drivers/net/wireless/prism54/isl_ioctl.h  |1 
 drivers/net/wireless/prism54/isl_oid.h|   32 ++---
 drivers/net/wireless/prism54/islpci_dev.c |   13 ++---
 drivers/net/wireless/prism54/islpci_dev.h |   11 ++--
 drivers/net/wireless/prism54/islpci_eth.c |7 +-
 drivers/net/wireless/prism54/islpci_eth.h |1 
 drivers/net/wireless/prism54/islpci_hotplug.c |   23 -
 drivers/net/wireless/prism54/islpci_mgt.c |3 -
 drivers/net/wireless/prism54/islpci_mgt.h |5 --
 drivers/net/wireless/prism54/oid_mgt.c|6 +-
 drivers/net/wireless/prism54/prismcompat.h|4 -
 14 files changed, 89 insertions(+), 102 deletions(-)

Index: work/drivers/net/wireless/prism54/isl_38xx.c
===
--- work.orig/drivers/net/wireless/prism54/isl_38xx.c
+++ work/drivers/net/wireless/prism54/isl_38xx.c
@@ -1,5 +1,4 @@
 /*
- *  
  *  Copyright (C) 2002 Intersil Americas Inc.
  *  Copyright (C) 2003-2004 Luis R. Rodriguez [EMAIL PROTECTED]_
  *
@@ -38,7 +37,7 @@
  * isl38xx_disable_interrupts - disable all interrupts
  * @device: pci memory base address
  *
- *  Instructs the device to disable all interrupt reporting by asserting 
+ *  Instructs the device to disable all interrupt reporting by asserting
  *  the IRQ line. New events may still show up in the interrupt identification
  *  register located at offset %ISL38XX_INT_IDENT_REG.
  */
@@ -204,17 +203,19 @@ isl38xx_interface_reset(void __iomem *de
/* enable the interrupt for detecting initialization */
 
/* Note: Do not enable other interrupts here. We want the
-* device to have come up first 100% before allowing any other 
+* device to have come up first 100% before allowing any other
 * interrupts. */
isl38xx_w32_flush(device_base, ISL38XX_INT_IDENT_INIT, 
ISL38XX_INT_EN_REG);
udelay(ISL38XX_WRITEIO_DELAY);  /* allow complete full reset */
 }
 
 void
-isl38xx_enable_common_interrupts(void __iomem *device_base) {
+isl38xx_enable_common_interrupts(void __iomem *device_base)
+{
u32 reg;
-   reg = ( ISL38XX_INT_IDENT_UPDATE | 
-   ISL38XX_INT_IDENT_SLEEP | ISL38XX_INT_IDENT_WAKEUP);
+
+   reg = ISL38XX_INT_IDENT_UPDATE | ISL38XX_INT_IDENT_SLEEP |
+ ISL38XX_INT_IDENT_WAKEUP;
isl38xx_w32_flush(device_base, reg, ISL38XX_INT_EN_REG);
udelay(ISL38XX_WRITEIO_DELAY);
 }
@@ -234,23 +235,21 @@ isl38xx_in_queue(isl38xx_control_block *
/* send queues */
case ISL38XX_CB_TX_MGMTQ:
BUG_ON(delta  ISL38XX_CB_MGMT_QSIZE);
+
case ISL38XX_CB_TX_DATA_LQ:
case ISL38XX_CB_TX_DATA_HQ:
BUG_ON(delta  ISL38XX_CB_TX_QSIZE);
return delta;
-   break;
 
/* receive queues */
case ISL38XX_CB_RX_MGMTQ:
BUG_ON(delta  ISL38XX_CB_MGMT_QSIZE);
return ISL38XX_CB_MGMT_QSIZE - delta;
-   break;
 
case ISL38XX_CB_RX_DATA_LQ:
case ISL38XX_CB_RX_DATA_HQ:
BUG_ON(delta  ISL38XX_CB_RX_QSIZE);
return ISL38XX_CB_RX_QSIZE - delta;
-   break;
}
BUG();
return 0;
Index: work/drivers/net/wireless/prism54/isl_38xx.h
===
--- work.orig/drivers/net/wireless/prism54/isl_38xx.h
+++ work/drivers/net/wireless/prism54/isl_38xx.h
@@ -1,5 +1,4 @@
 /*
- *  
  *  Copyright (C) 2002 Intersil Americas Inc.
  *
  *  This program is free software; you can redistribute it and/or modify
@@ -67,10 +66,10 @@
  * @base: (host) memory base address of the device
  * @val: 32bit value (host order) to write
  * @offset: byte offset into @base to write value to
- * 
+ *
  *  This helper takes care of writing a 32bit datum to the
- *  specified offset into the device's pci memory space, and making sure 
- *  the pci memory buffers get flushed by performing one harmless read 
+ *  specified offset into the device's pci memory space, and making sure
+ *  the pci memory buffers get flushed by performing one harmless read
  *  from the %ISL38XX_PCI_POSTING_FLUSH offset.
  */
 static inline void
Index: work/drivers/net/wireless/prism54/isl_ioctl.c
===
--- work.orig/drivers/net/wireless/prism54/isl_ioctl.c
+++ work/drivers/net/wireless/prism54/isl_ioctl.c
@@ -1,5 +1,4 @@
 /*
- *  
  *  Copyright (C) 2002 Intersil Americas Inc.
  *(C) 2003,2004 Aurelien Alleaume [EMAIL PROTECTED]
  *(C) 2003 Herbert Valerio Riedel [EMAIL PROTECTED

Re: 2.6.18-rc7-mm1: networking breakage on HPC nx6325 + SUSE 10.1

[Dmitry Torokhov]

On Tuesday 19 September 2006 18:30, Greg KH wrote:
 On Tue, Sep 19, 2006 at 03:06:29PM -0700, David Miller wrote:
  From: Rafael J. Wysocki [EMAIL PROTECTED]
  Date: Wed, 20 Sep 2006 00:06:52 +0200
  
   I _guess_ the problem is caused by
   gregkh-driver-network-class_device-to-device.patch, but I can't verify 
   this,
   because the kernel (obviously) doesn't compile if I revert it.
  
  Indeed.
  
  I thought we threw this patch out because we knew it would cause
  problems for existing systems?  I do remember Greg making an argument
  as to why we needed the change, but that doesn't make breaking people's
  systems legitimate in any way.
 
 It's now thrown out, and I think Andrew already had a patch in his tree
 that reverted this.
 
 I'll be bringing it back eventually, but first we are going to work out
 all the kinks by probably putting these changes in the next few SuSE
 alpha releases to see what shakes out in userspace that we need to go
 fix.
 

Greg,

Could you please comment on the patch below which is I believe achieves
the desired result - produces unified sysfs representation of kernel
device tree - without major reshuffle of every kernel subsystem.

-- 
Dmitry


Driver core: move class_device to /sys/device/... part of the tree

Move sysfs representation of class_device structure from /sys/class/...
to /sys/device/... to provide unified device tree; create symlinks
in /sys/class pointing to /sys/device/... to preserve existing
classification of devices.

Create /sys/device/virtual device which is parent for all class_devices
that do not have real parent device.

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/base/class.c |  154 ---
 1 files changed, 73 insertions(+), 81 deletions(-)

Index: work/drivers/base/class.c
===
--- work.orig/drivers/base/class.c
+++ work/drivers/base/class.c
@@ -521,60 +521,73 @@ char *make_class_name(const char *name, 
return class_name;
 }
 
+static struct device virtual_device = {
+   .bus_id = virtual,
+};
+
 int class_device_add(struct class_device *class_dev)
 {
-   struct class *parent_class = NULL;
+   struct class *parent_class;
struct class_device *parent_class_dev = NULL;
+   struct device *parent_dev = NULL;
struct class_interface *class_intf;
-   char *class_name = NULL;
int error = -EINVAL;
 
-   class_dev = class_device_get(class_dev);
-   if (!class_dev)
-   return -EINVAL;
-
if (!strlen(class_dev-class_id))
-   goto out1;
+   return -EINVAL;
 
parent_class = class_get(class_dev-class);
if (!parent_class)
-   goto out1;
-
-   parent_class_dev = class_device_get(class_dev-parent);
+   return -EINVAL;
 
pr_debug(CLASS: registering class device: ID = '%s'\n,
 class_dev-class_id);
 
+   parent_class_dev = class_device_get(class_dev-parent);
+
+   if (!class_dev-dev)
+   class_dev-dev = virtual_device;
+   parent_dev = get_device(class_dev-dev);
+
/* first, register with generic layer. */
error = kobject_set_name(class_dev-kobj, %s, class_dev-class_id);
if (error)
-   goto out2;
+   goto err_put_parents;
 
-   if (parent_class_dev)
-   class_dev-kobj.parent = parent_class_dev-kobj;
-   else
-   class_dev-kobj.parent = parent_class-subsys.kset.kobj;
+   class_dev-kobj.parent = parent_class_dev ?
+   parent_class_dev-kobj : parent_dev-kobj;
 
error = kobject_add(class_dev-kobj);
if (error)
-   goto out2;
+   goto err_put_parents;
 
/* add the needed attributes to this device */
-   sysfs_create_link(class_dev-kobj, parent_class-subsys.kset.kobj, 
subsystem);
+   error = sysfs_create_link(class_dev-kobj,
+ parent_class-subsys.kset.kobj,
+ subsystem);
+   if (error)
+   goto err_del_kobject;
+
+   error = sysfs_create_link(parent_class-subsys.kset.kobj,
+ class_dev-kobj,
+ class_dev-class_id);
+   if (error)
+   goto err_del_subsys_link;
+
class_dev-uevent_attr.attr.name = uevent;
class_dev-uevent_attr.attr.mode = S_IWUSR;
class_dev-uevent_attr.attr.owner = parent_class-owner;
class_dev-uevent_attr.store = store_uevent;
error = class_device_create_file(class_dev, class_dev-uevent_attr);
if (error)
-   goto out3;
+   goto err_del_class_link;
 
if (MAJOR(class_dev-devt)) {
struct class_device_attribute *attr;
attr = kzalloc(sizeof(*attr), GFP_KERNEL);
if (!attr) {
error

Re: [patch 8/9] smsc-ircc2: fix section reference mismatches

[Dmitry Torokhov]

On 7/7/06, Jeff Garzik [EMAIL PROTECTED] wrote:

[EMAIL PROTECTED] wrote:
 From: Dmitry Torokhov [EMAIL PROTECTED]

 subsystem_configurations array is only used by an __init function,
 therefore it should be marked __initdata, not __devinitdata.

 Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
 Signed-off-by: Andrew Morton [EMAIL PROTECTED]
 ---

  drivers/net/irda/smsc-ircc2.c |2 +-
  1 files changed, 1 insertion(+), 1 deletion(-)

 diff -puN 
drivers/net/irda/smsc-ircc2.c~smsc-ircc2-fix-section-reference-mismatches 
drivers/net/irda/smsc-ircc2.c
 --- 
a/drivers/net/irda/smsc-ircc2.c~smsc-ircc2-fix-section-reference-mismatches
 +++ a/drivers/net/irda/smsc-ircc2.c
 @@ -2353,7 +2353,7 @@ static int __init smsc_superio_lpc(unsig
  #ifdef CONFIG_PCI
  #define PCIID_VENDOR_INTEL 0x8086
  #define PCIID_VENDOR_ALI 0x10b9
 -static struct smsc_ircc_subsystem_configuration subsystem_configurations[] 
__devinitdata = {
 +static struct smsc_ircc_subsystem_configuration subsystem_configurations[] 
__initdata = {

Are you sure this fully solves the problem?



I think so, at least I see no more warnigns...


It seems like the functions referenced inside this are inappropriately
marked as well...



subsystem_configurations is only used from
smsc_ircc_preconfigure_subsystems(), which is called from
smsc_ircc_init(). Both of these are __init. All methods in
subsystem_configurations are marked as __init as well. Seems logical
that the strcture should be marked __initdata.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/3] atmel: save on array initialization

[Dmitry Torokhov]

On 12/1/05, Jeff Garzik [EMAIL PROTECTED] wrote:
 Dmitry Torokhov wrote:
  NET: atmel - do not initialize array over and over again
 
  Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]

 can you resend this series without MIME attachments?


Jeff,

I would love to but there was no traces of MIME in my messages.
Straight 7 bit ascii, nothing fancy.

--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Mouse issues in -mm

[Dmitry Torokhov]

On Thursday 24 November 2005 01:14, Frank Sorenson wrote:
 Note: I believe this issue may also be related to the mouse protocol
 extension.  I typically run with 'psmouse.proto=exps' on the kernel
 command line, and the psmouse resync patch seems to break tapping in
 that mode.

So the resync patch - is it only tapping that is broken or you also
loosing the touchpad completely?

 However, booting without proto=exps seems to continue to 
 work, even with the resync patch (though the touchpad is unusably
 sensitive--hence the use of exps in the first place).

Without the parameter your touchpad is using native ALPS protocol and
resync is disabled for it. You may have better liuck with toucpad
sensitivity using synaptics X driver.

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Mouse issues in -mm

[Dmitry Torokhov]

On Wednesday 23 November 2005 21:06, Frank Sorenson wrote:
 Andrew Morton wrote:
  Marc Koschewski [EMAIL PROTECTED] wrote:
 Just booted into 2.6.15-rc2-mm1. The 'mouse problem' (as reported earlier) 
 still
 persists,
 
  You'l probably need to re-report the mouse problem if the previous report
  didn't get any action.
 
 I'm not certain whether this is the same 'mouse problem', but I'll
 report the mouse problems I've been seeing.  In the past several -mm
 kernels, my touchpad has initially worked at boot, but 'tapping' has
 stopped working at some point later (with no obvious kernel messages).
 
 I've experienced this problem at least with 2.6.15-rc1-mm2 and
 2.6.15-rc2-mm1, and reverting
 input-attempt-to-re-synchronize-mouse-every-5-seconds.patch gives a
 kernel without the touchpad problems.


What kind of touchpad do you have? Could you post your
/pros/bus/input/devices please?

-- 
Dmitry
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 1/3] atmel: save on array initialization

[Dmitry Torokhov]

NET: atmel - do not initialize array over and over again

Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---

 drivers/net/wireless/atmel.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Index: work/drivers/net/wireless/atmel.c
===
--- work.orig/drivers/net/wireless/atmel.c
+++ work/drivers/net/wireless/atmel.c
@@ -826,12 +826,12 @@ static void tx_update_descriptor(struct 
 
 static int start_tx (struct sk_buff *skb, struct net_device *dev)
 {
+   static const u8 SNAP_RFC1024[6] = { 0xaa, 0xaa, 0x03, 0x00, 0x00, 0x00 
};
struct atmel_private *priv = netdev_priv(dev);
struct ieee80211_hdr_4addr header;
unsigned long flags;
u16 buff, frame_ctl, len = (ETH_ZLEN  skb-len) ? skb-len : ETH_ZLEN;
-   u8 SNAP_RFC1024[6] = {0xaa, 0xaa, 0x03, 0x00, 0x00, 0x00};
- 
+
if (priv-card  priv-present_callback  
!(*priv-present_callback)(priv-card)) {
priv-stats.tx_errors++;

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html