[PATCH 1/2] HID: add hid_type

[PATCH 1/2] HID: add hid_type

[Jiri Slaby]

Add type to the hid structure to distinguish to which device type
(mouse/kbd) we are talking to. Needed for per device type ignore
list support.

Note: this patch leaves the type as unknown for bluetooth devices,
there is not support for this in the hidp code.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/hid/usbhid/hid-core.c |    8 ++++++++
 include/linux/hid.h           |    7 +++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1d3b8a3..20617d8 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -972,6 +972,14 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
 	hid->product = le16_to_cpu(dev->descriptor.idProduct);
 	hid->name[0] = 0;
+	switch (intf->cur_altsetting->desc.bInterfaceProtocol) {
+	case USB_INTERFACE_PROTOCOL_KEYBOARD:
+		hid->type = HID_TYPE_KEYBOARD;
+		break;
+	case USB_INTERFACE_PROTOCOL_MOUSE:
+		hid->type = HID_TYPE_MOUSE;
+		break;
+	}
 
 	if (dev->manufacturer)
 		strlcpy(hid->name, dev->manufacturer, sizeof(hid->name));
diff --git a/include/linux/hid.h b/include/linux/hid.h
index f13bca2..36a3953 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -417,6 +417,12 @@ struct hid_input {
 	struct input_dev *input;
 };
 
+enum hid_type {
+	HID_TYPE_UNKNOWN = 0,
+	HID_TYPE_MOUSE,
+	HID_TYPE_KEYBOARD
+};
+
 struct hid_driver;
 struct hid_ll_driver;
 
@@ -431,6 +437,7 @@ struct hid_device {							/* device report descriptor */
 	__u32 vendor;							/* Vendor ID */
 	__u32 product;							/* Product ID */
 	__u32 version;							/* HID version */
+	enum hid_type type;						/* device type (mouse, kbd, ...) */
 	unsigned country;						/* HID country */
 	struct hid_report_enum report_enum[HID_REPORT_TYPES];
 
-- 
1.6.0.2

[PATCH 2/2] HID: fix appletouch regression

[Jiri Slaby]

The appletouch mouse devices are grabbed by the hid bus and not
released even if apple driver says ENODEV (as expected).

Move the ignoration one level upper to deny the hid layer to grab
the device and return error to the usb hid which, as a result,
releases the device.

Otherwise input/mouse/appletouch and others needn't be attached.

Reported-by: Justin Mattock <justinmattock@gmail.com>
Reported-by: Steven Noonan <steven@uplinklabs.net>
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/hid/hid-apple.c |   63 ++++++++++++++++------------------------------
 drivers/hid/hid-core.c  |   33 ++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index fd7f896..c6ab4ba 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -312,13 +312,6 @@ static int apple_probe(struct hid_device *hdev,
 	unsigned int connect_mask = HID_CONNECT_DEFAULT;
 	int ret;
 
-	/* return something else or move to hid layer? device will reside
-	   allocated */
-	if (id->bus == BUS_USB && (quirks & APPLE_IGNORE_MOUSE) &&
-			to_usb_interface(hdev->dev.parent)->cur_altsetting->
-			desc.bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE)
-		return -ENODEV;
-
 	asc = kzalloc(sizeof(*asc), GFP_KERNEL);
 	if (asc == NULL) {
 		dev_err(&hdev->dev, "can't alloc apple descriptor\n");
@@ -367,38 +360,32 @@ static const struct hid_device_id apple_devices[] = {
 		.driver_data = APPLE_MIGHTYMOUSE | APPLE_INVERT_HWHEEL },
 
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_JIS),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS},
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ANSI),
 		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO),
@@ -406,14 +393,12 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS),
 		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
@@ -422,25 +407,21 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 
 	/* Apple wireless Mighty Mouse */
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, 0x030c),
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8a7d9db..90bdc6f 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1539,6 +1539,35 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ }
 };
 
+/**
+ * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
+ */
+static const struct hid_device_id hid_mouse_ignore_list[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) },
+	{ }
+};
+
 static bool hid_ignore(struct hid_device *hdev)
 {
 	switch (hdev->vendor) {
@@ -1555,6 +1584,10 @@ static bool hid_ignore(struct hid_device *hdev)
 		break;
 	}
 
+	if (hdev->type == HID_TYPE_MOUSE &&
+			hid_match_id(hdev, hid_mouse_ignore_list))
+		return true;
+
 	return !!hid_match_id(hdev, hid_ignore_list);
 }
 
-- 
1.6.0.2

Re: [PATCH 1/2] HID: add hid_type

[Justin Mattock]

On Sun, Oct 19, 2008 at 7:15 AM, Jiri Slaby <jirislaby@gmail.com> wrote:
> Add type to the hid structure to distinguish to which device type
> (mouse/kbd) we are talking to. Needed for per device type ignore
> list support.
>
> Note: this patch leaves the type as unknown for bluetooth devices,
> there is not support for this in the hidp code.
>
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> ---
>  drivers/hid/usbhid/hid-core.c |    8 ++++++++
>  include/linux/hid.h           |    7 +++++++
>  2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 1d3b8a3..20617d8 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -972,6 +972,14 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
>        hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
>        hid->product = le16_to_cpu(dev->descriptor.idProduct);
>        hid->name[0] = 0;
> +       switch (intf->cur_altsetting->desc.bInterfaceProtocol) {
> +       case USB_INTERFACE_PROTOCOL_KEYBOARD:
> +               hid->type = HID_TYPE_KEYBOARD;
> +               break;
> +       case USB_INTERFACE_PROTOCOL_MOUSE:
> +               hid->type = HID_TYPE_MOUSE;
> +               break;
> +       }
>
>        if (dev->manufacturer)
>                strlcpy(hid->name, dev->manufacturer, sizeof(hid->name));
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f13bca2..36a3953 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -417,6 +417,12 @@ struct hid_input {
>        struct input_dev *input;
>  };
>
> +enum hid_type {
> +       HID_TYPE_UNKNOWN = 0,
> +       HID_TYPE_MOUSE,
> +       HID_TYPE_KEYBOARD
> +};
> +
>  struct hid_driver;
>  struct hid_ll_driver;
>
> @@ -431,6 +437,7 @@ struct hid_device {                                                 /* device report descriptor */
>        __u32 vendor;                                                   /* Vendor ID */
>        __u32 product;                                                  /* Product ID */
>        __u32 version;                                                  /* HID version */
> +       enum hid_type type;                                             /* device type (mouse, kbd, ...) */
>        unsigned country;                                               /* HID country */
>        struct hid_report_enum report_enum[HID_REPORT_TYPES];
>
> --
> 1.6.0.2
>
>

O.k. reverted the old patch and
applied the two new ones.
appletouchpad is working;
I didn't use patch -p1,
just manually applied the two.

-- 
Justin P. Mattock

Re: several messages

[Jiri Kosina]

On Sun, 19 Oct 2008, Jiri Slaby wrote:

> +enum hid_type {
> +	HID_TYPE_UNKNOWN = 0,
> +	HID_TYPE_MOUSE,
> +	HID_TYPE_KEYBOARD
> +};
> +

Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in 
the code. I'd propose to add it when it is actually needed. I.e. have the 
enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add 
whatever will become necessary in the future, what do you think?


On Sun, 19 Oct 2008, Jiri Slaby wrote:

> +/**
> + * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
> + */

I think a more descriptive comment would be appropriate here. It might not 
be obvious on the first sight why this needs to be done separately from 
the generic hid_blacklist. I.e. something like

/** 
 * There are composite devices for which we want to ignore only a certain
 * interface. This is a list of devices for which only the mouse interface 
 * will be ignored.
 */

maybe?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: several messages

[Justin Mattock]

On Sun, Oct 19, 2008 at 12:40 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
>
>> +enum hid_type {
>> +     HID_TYPE_UNKNOWN = 0,
>> +     HID_TYPE_MOUSE,
>> +     HID_TYPE_KEYBOARD
>> +};
>> +
>
> Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in
> the code. I'd propose to add it when it is actually needed. I.e. have the
> enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add
> whatever will become necessary in the future, what do you think?
>
>
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
>
>> +/**
>> + * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
>> + */
>
> I think a more descriptive comment would be appropriate here. It might not
> be obvious on the first sight why this needs to be done separately from
> the generic hid_blacklist. I.e. something like
>
> /**
>  * There are composite devices for which we want to ignore only a certain
>  * interface. This is a list of devices for which only the mouse interface
>  * will be ignored.
>  */
>
> maybe?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

I can agree with that, whats the point having something
there if it not being used,(just eating up precious space);

-- 
Justin P. Mattock

Re: several messages

[Jiri Slaby]

Jiri Kosina napsal(a):
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
> 
>> +enum hid_type {
>> +	HID_TYPE_UNKNOWN = 0,
>> +	HID_TYPE_MOUSE,
>> +	HID_TYPE_KEYBOARD
>> +};
>> +
> 
> Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in 
> the code. I'd propose to add it when it is actually needed. I.e. have the 
> enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add 
> whatever will become necessary in the future, what do you think?

I would use unknown rather than other, since on bluetooth mouse is unknown
not other, if you don't mind?

Or did you mean tristate unknown, mouse and other?

Thanks for review.

[PATCH 2/2] HID: fix appletouch regression (was Re: several messages)

[Jiri Kosina]

[ restored previous subject, sorry for the "several messagess" mess, 
alpine didn't handle multi-message reply with different subjects nicely ]

On Mon, 20 Oct 2008, Jiri Slaby wrote:

> >> +enum hid_type {
> >> +	HID_TYPE_UNKNOWN = 0,
> >> +	HID_TYPE_MOUSE,
> >> +	HID_TYPE_KEYBOARD
> > Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere 
> > in the code. I'd propose to add it when it is actually needed. I.e. 
> > have the enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for 
> > now, and add whatever will become necessary in the future, what do you 
> > think?
> I would use unknown rather than other, since on bluetooth mouse is unknown
> not other, if you don't mind?
> Or did you mean tristate unknown, mouse and other?

Well, basically the points I am trying to make:

1) the code doesn't currently need HID_TYPE_KEYBOARD at all

and

2) by definition, HID are not just mice and keyboards, so adding just 
   these two doesn't look particularly correct, if just one of them is 
   needed

For bluetooth we even don't have the possibility to distinguish these, 
yes. So probably something like HID_TYPE_USBMOUSE/HID_TYPE_OTHER for now, 
and we could extend the enum whenever the need to recognize a different 
type comes up?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: several messages

[Borislav Petkov]

On Wed, Jan 27, 2016 at 11:09:04AM +0100, Thomas Gleixner wrote:
> On Mon, 25 Jan 2016, Andy Lutomirski wrote:
> > This is a straightforward speedup on Ivy Bridge and newer, IIRC.
> > (I tested on Skylake.  INVPCID is not available on Sandy Bridge.
> > I don't have Ivy Bridge, Haswell or Broadwell to test on, so I
> > could be wrong as to when the feature was introduced.)
>
> Haswell and Broadwell have it. No idea about ivy bridge.

I have an IVB model 58. It doesn't have it:

CPUID_0x00000007: EAX=0x00000000, EBX=0x00000281, ECX=0x00000000, EDX=0x00000000

INVPCID should be EBX[10].

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: several messages

[Thomas Gleixner]

On Mon, 25 Jan 2016, Andy Lutomirski wrote:
> This is a straightforward speedup on Ivy Bridge and newer, IIRC.
> (I tested on Skylake.  INVPCID is not available on Sandy Bridge.
> I don't have Ivy Bridge, Haswell or Broadwell to test on, so I
> could be wrong as to when the feature was introduced.)

Haswell and Broadwell have it. No idea about ivy bridge.
 
Thanks,

	tglx

RE: several messages

[Wu, Feng]

> -----Original Message-----
> From: Jiang Liu [mailto:jiang.liu@linux.intel.com]
> Sent: Tuesday, November 11, 2014 10:29 AM
> To: Thomas Gleixner; Wu, Feng
> Cc: gleb@kernel.org; pbonzini@redhat.com; David Woodhouse;
> joro@8bytes.org; mingo@redhat.com; H. Peter Anvin; x86@kernel.org;
> kvm@vger.kernel.org; iommu@lists.linux-foundation.org; LKML
> Subject: Re: several messages
> 
> On 2014/11/11 2:15, Thomas Gleixner wrote:
> > On Mon, 10 Nov 2014, Feng Wu wrote:
> >
> >> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
> >> With VT-d Posted-Interrupts enabled, external interrupts from
> >> direct-assigned devices can be delivered to guests without VMM
> >> intervention when guest is running in non-root mode.
> >
> > Can you please talk to Jiang and synchronize your work with his
> > refactoring of the x86 interrupt handling subsystem.
> >
> > I want this stuff cleaned up first before we add new stuff to it.
> Hi Thomas,
> 	Just talked with Feng, we will focused on refactor first and
> then add posted interrupt support.
> Regards!
> Gerry

No problem!

Thanks,
Feng

> 
> >
> > Thanks,
> >
> > 	tglx
> >

Re: several messages

[Jiang Liu]

On 2014/11/11 2:15, Thomas Gleixner wrote:
> On Mon, 10 Nov 2014, Feng Wu wrote:
> 
>> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
>> With VT-d Posted-Interrupts enabled, external interrupts from
>> direct-assigned devices can be delivered to guests without VMM
>> intervention when guest is running in non-root mode.
> 
> Can you please talk to Jiang and synchronize your work with his
> refactoring of the x86 interrupt handling subsystem.
> 
> I want this stuff cleaned up first before we add new stuff to it.
Hi Thomas,
	Just talked with Feng, we will focused on refactor first and
then add posted interrupt support.
Regards!
Gerry

> 
> Thanks,
> 
> 	tglx
> 

Re: several messages

[Thomas Gleixner]

On Mon, 10 Nov 2014, Feng Wu wrote:

> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
> With VT-d Posted-Interrupts enabled, external interrupts from
> direct-assigned devices can be delivered to guests without VMM
> intervention when guest is running in non-root mode.

Can you please talk to Jiang and synchronize your work with his
refactoring of the x86 interrupt handling subsystem.

I want this stuff cleaned up first before we add new stuff to it.

Thanks,

	tglx

Re: several messages

[Jan Engelhardt]

On Thursday 2010-07-15 11:36, Patrick McHardy wrote:
>> +struct xt_CHECKSUM_info {
>> +	u_int8_t operation;	/* bitset of operations */
>
>Please use __u8 in public header files.
>
>> +#include <linux/in.h>
>
>Do you really need in.h?
>
>> + * $Id$
>
>Please no CVS IDs.
>
>> +	switch (c) {
>> +	case 'F':
>> +		if (*flags)
>> +			xtables_error(PARAMETER_PROBLEM,
>> +			        "CHECKSUM target: Only use --checksum-fill ONCE!");
>
>There is a helper function called xtables_param_act for checking double
>arguments and printing a standarized error message.

I took care of these for Xt-a.

Re: several messages

[Jiri Kosina]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 406 bytes --]

On Wed, 26 Nov 2008, J.R. Mauro wrote:

> There is one bluetooth model and one USB model.

On Wed, 26 Nov 2008, Tobias Müller wrote:

> I own the USB variant and these are the right id for that. The wireless 
> IDs were from another patch I merged together with mine. I don't have a 
> wireless version.

OK, so therefore you confirm that Jan's patch is OK, right?

Thanks a lot,

-- 
Jiri Kosina
SUSE Labs

Re: several messages

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

On Thu, 2007-02-22 at 23:58 +0100, Guennadi Liakhovetski wrote:

> I think, we only want 1, right? And the latter seems to be more generic / 
> platform independent? And as a side-effect, powermac would have to migrate 
> to generic rtc:-)

Can we migrate all of powerpc to genrtc? But yes, I agree. Had enough to
do though already to get suspend working :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: several messages

[David Brownell]

On Thursday 22 February 2007 2:58 pm, Guennadi Liakhovetski wrote:
> 
> I think, we only want 1, right? And the latter seems to be more generic / 
> platform independent? And as a side-effect, powermac would have to migrate 
> to generic rtc:-)

I'd certainly think that restoring the system clock should be, as much
as possible, in platform-agnostic code.  Like the generic RTC framework.

And hmm, that powermac/time.c file replicates other RTC code...

Minor obstacle:  removing the EXPERIMENTAL label from that code.

- Dave

Re: several messages

[Guennadi Liakhovetski]

of the following 2 patches:

On Mon, 5 Feb 2007, Johannes Berg wrote:

> This patch removes the time suspend/restore code that was done through
> a PMU notifier in arch/platforms/powermac/time.c.
> 
> Instead, we introduce arch/powerpc/sysdev/timer.c which creates a sys
> device and handles time of day suspend/resume through that.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> Cc: Andrew Morton <akpm@osdl.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

[patch trimmed]

On Wed, 21 Feb 2007, David Brownell wrote:

> RTC class suspend/resume support, re-initializing the system clock on resume
> >from the clock used to initialize it at boot time.
> 
>  - Inlining the same code used by ARM, which saves and restores the
>    delta between a selected RTC and the current system wall-clock time.
> 
>  - Removes calls to that ARM code from AT91, OMAP, and S3C RTCs.
> 
> This goes on top of the patch series removing "struct class_device" usage
> >from the RTC framework.  That makes class suspend()/resume() work.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> 
> ---
>  drivers/rtc/Kconfig          |   24 +++++++++----
>  drivers/rtc/class.c          |   74 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/rtc/rtc-at91rm9200.c |   30 -----------------
>  drivers/rtc/rtc-omap.c       |   17 ---------
>  drivers/rtc/rtc-s3c.c        |   22 ------------
>  5 files changed, 91 insertions(+), 76 deletions(-)

[patch trimmed]

I think, we only want 1, right? And the latter seems to be more generic / 
platform independent? And as a side-effect, powermac would have to migrate 
to generic rtc:-)

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: several messages

[Nix]

On 11 Apr 2006, Greg KH whispered secretively:
> The first one went out last night, as it was a real issue that affected
> people and I had already waited longer than I felt comfortable with, due
> to travel issues I had (two different talks in two different cities in
> two different days.)
> 
> The second one went out today, because it was reported today.  Should I
> have waited until tomorrow to see if something else came up?

Indeed.

On top of that, they're `only' local DoSes, so many admins (i.e. those
without untrusted local users) will probably not have installed .3 yet:
and anyone with untrusted local users probably has someone whose entire
job is handling security anyway.


There's nothing wrong with rapid-fire -stables; either the issue is (in
the judgement of the ones doing the installation) critical, in which
case it should get out as fast as possible, or it isn't, in which case
the local installing admins can put it off for a day or so themselves to
see if another release comes out immediately afterwards.

-- 
`On a scale of 1-10, X's "brokenness rating" is 1.1, but that's only
 because bringing Windows into the picture rescaled "brokenness" by
 a factor of 10.' --- Peter da Silva

Re: several messages

[Jan Engelhardt]

>
>The first one went out last night, as it was a real issue that affected
>people and I had already waited longer than I felt comfortable with, due
>to travel issues I had (two different talks in two different cities in
>two different days.)
>
>The second one went out today, because it was reported today.  Should I
>have waited until tomorrow to see if something else came up?
>
No of course not, I did not know the first one was already due long time.


[Sigh, pine changed the subject header and I did not notice.]


Jan Engelhardt
-- 

Re: several messages

[Greg KH]

On Tue, Apr 11, 2006 at 09:04:42PM +0200, Jan Engelhardt wrote:
> 
> >Date: Tue, 11 Apr 2006 09:26:20 -0700
> >Subject: Linux 2.6.16.3
> >David Howells:
> >      Keys: Fix oops when adding key to non-keyring [CVE-2006-1522]
> 
> >Date: Tue, 11 Apr 2006 10:33:23 -0700
> >Subject: Linux 2.6.16.4
> >Oleg Nesterov:
> >      RCU signal handling [CVE-2006-1523]
> 
> Now admins spend another hour this day just to upgrade.
> These two patches could have been queued until the end of the day. Maybe 
> another one turns up soon.

The first one went out last night, as it was a real issue that affected
people and I had already waited longer than I felt comfortable with, due
to travel issues I had (two different talks in two different cities in
two different days.)

The second one went out today, because it was reported today.  Should I
have waited until tomorrow to see if something else came up?

thanks,

greg k-h

Re: several messages

[Boris B. Zhmurov]

Hello, Jan Engelhardt.

On 11.04.2006 23:04 you said the following:


> Now admins spend another hour this day just to upgrade.

It's admin's job, isn't it?


> These two patches could have been queued until the end of the day. Maybe 
> another one turns up soon.
> Jan Engelhardt

Hmm... Interesting. Are you blaming security officers for doing their 
job? Please, don't! And many many thanks to Greg for giving us security 
patches as soon as possible.


-- 
Boris B. Zhmurov
mailto: bb@kernelpanic.ru
"wget http://kernelpanic.ru/bb_public_key.pgp -O - | gpg --import"

Re: several messages

[Jan Engelhardt]

>Date: Tue, 11 Apr 2006 09:26:20 -0700
>Subject: Linux 2.6.16.3
>David Howells:
>      Keys: Fix oops when adding key to non-keyring [CVE-2006-1522]

>Date: Tue, 11 Apr 2006 10:33:23 -0700
>Subject: Linux 2.6.16.4
>Oleg Nesterov:
>      RCU signal handling [CVE-2006-1523]

Now admins spend another hour this day just to upgrade.
These two patches could have been queued until the end of the day. Maybe 
another one turns up soon.


Jan Engelhardt
-- 

Re: several messages

[Dmitry Yusupov]

On Fri, 2005-05-13 at 16:07 +0100, Christoph Hellwig wrote:
> On Fri, May 13, 2005 at 08:04:16AM -0700, Dmitry Yusupov wrote:
> > You could tell this to school's computer class teacher... Serious SAN
> > deployment will always be based either on FC or iSCSI for the reasons I
> > explained before.
> 
> Just FYI Steeleye ships a very successful clustering product that builds
> on nbd.

AFAIK, it is used for Data Replication purposes only. Correct me if I'm
wrong...

Re: several messages

[Christoph Hellwig]

On Fri, May 13, 2005 at 08:04:16AM -0700, Dmitry Yusupov wrote:
> You could tell this to school's computer class teacher... Serious SAN
> deployment will always be based either on FC or iSCSI for the reasons I
> explained before.

Just FYI Steeleye ships a very successful clustering product that builds
on nbd.

Re: several messages

[Dmitry Yusupov]

On Fri, 2005-05-13 at 09:12 +0100, Christoph Hellwig wrote:
> On Thu, May 12, 2005 at 11:32:12AM -0700, Dmitry Yusupov wrote:
> > On Wed, 2005-05-11 at 22:16 -0400, Ming Zhang wrote:
> > > iscsi is scsi over ip.
> > 
> > correction. iSCSI today has RFC at least for two transports - TCP/IP and
> > iSER/RDMA(in finalized progress) with RDMA over Infiniband or RNIC. And
> > I think people start writing initial draft for SCTP/IP transport...
> > 
> > >From this perspective, iSCSI certainly more advanced and matured
> > comparing to NBD variations. 
> 
> It's for certainly much more complicated (in marketing speak that's usually
> called advanced) but far less mature.
> 
> If you want network storage to just work use nbd.

You could tell this to school's computer class teacher... Serious SAN
deployment will always be based either on FC or iSCSI for the reasons I
explained before.

I do not disagree, nbd is nice and simple and for sure has its own
deployment space.

Dmitry

Re: several messages

[Christoph Hellwig]

On Thu, May 12, 2005 at 11:32:12AM -0700, Dmitry Yusupov wrote:
> On Wed, 2005-05-11 at 22:16 -0400, Ming Zhang wrote:
> > iscsi is scsi over ip.
> 
> correction. iSCSI today has RFC at least for two transports - TCP/IP and
> iSER/RDMA(in finalized progress) with RDMA over Infiniband or RNIC. And
> I think people start writing initial draft for SCTP/IP transport...
> 
> >From this perspective, iSCSI certainly more advanced and matured
> comparing to NBD variations. 

It's for certainly much more complicated (in marketing speak that's usually
called advanced) but far less mature.

If you want network storage to just work use nbd.

Re: several messages

[Dmitry Yusupov]

On Wed, 2005-05-11 at 22:16 -0400, Ming Zhang wrote:
> iscsi is scsi over ip.

correction. iSCSI today has RFC at least for two transports - TCP/IP and
iSER/RDMA(in finalized progress) with RDMA over Infiniband or RNIC. And
I think people start writing initial draft for SCTP/IP transport...

>From this perspective, iSCSI certainly more advanced and matured
comparing to NBD variations. 

> usb disk is scsi over usb.
> so just a different transport.
> u are rite. ;)
> 
> ming
> 
> On Wed, 2005-05-11 at 23:26 +0200, Guennadi Liakhovetski wrote:
> > Hello and thanks for the replies
> > 
> > On Wed, 11 May 2005, FUJITA Tomonori wrote:
> > > The iSCSI protocol simply encapsulates the SCSI protocol into the
> > > TCP/IP protocol, and carries packets over IP networks. You can handle
> > ...
> > 
> > On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> > > Actually, this is property not of iSCSI target itself, but of any SCSI target.
> > > So, we implemented it as part of our SCSI target mid-level (SCST,
> > > http://scst.sourceforge.net), therefore any target driver working over it will
> > > automatically benefit from this feature. Unfortunately, currently available
> > > only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
> > > works not too reliable and only with very specific initiators). The published
> > ...
> > 
> > The above confirms basically my understanding apart from one "minor" 
> > confusion - I thought, that parallel to hardware solutions pure software 
> > implementations were possible / being developed, like a driver, that 
> > implements a SCSI LDD API on one side, and forwards packets to an IP 
> > stack, say, over an ethernet card - on the initiator side. And a counter 
> > part on the target side. Similarly to the USB mass-storage and storage 
> > gadget drivers?
> > 
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski
> > 
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: several messages

[Vladislav Bolkhovitin]

Guennadi Liakhovetski wrote:
> Hello and thanks for the replies
> 
> On Wed, 11 May 2005, FUJITA Tomonori wrote:
> 
>>The iSCSI protocol simply encapsulates the SCSI protocol into the
>>TCP/IP protocol, and carries packets over IP networks. You can handle
> 
> ...
> 
> On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> 
>>Actually, this is property not of iSCSI target itself, but of any SCSI target.
>>So, we implemented it as part of our SCSI target mid-level (SCST,
>>http://scst.sourceforge.net), therefore any target driver working over it will
>>automatically benefit from this feature. Unfortunately, currently available
>>only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
>>works not too reliable and only with very specific initiators). The published
> 
> ...
> 
> The above confirms basically my understanding apart from one "minor" 
> confusion - I thought, that parallel to hardware solutions pure software 
> implementations were possible / being developed, like a driver, that 
> implements a SCSI LDD API on one side, and forwards packets to an IP 
> stack, say, over an ethernet card - on the initiator side. And a counter 
> part on the target side. Similarly to the USB mass-storage and storage 
> gadget drivers?

There is some confusion in the SCSI world between SCSI as a transport 
and SCSI as a commands set and software communication protocol, which 
works above the transport. So, you can implement SCSI transport at any 
software (eg iSCSI) or hardware (parallel SCSI, Fibre Channel, SATA, 
etc.) way, but if the SCSI message passing protocol is used overall 
system remains SCSI with all protocol obligations like task management. 
So, pure software SCSI solution is possible. BTW, there are pure 
hardware iSCSI implementations as well.

Vlad

> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

Re: several messages

[Ming Zhang]

[-- Attachment #1: Type: text/plain, Size: 1703 bytes --]

iscsi is scsi over ip.
usb disk is scsi over usb.
so just a different transport.
u are rite. ;)

ming

On Wed, 2005-05-11 at 23:26 +0200, Guennadi Liakhovetski wrote:
> Hello and thanks for the replies
> 
> On Wed, 11 May 2005, FUJITA Tomonori wrote:
> > The iSCSI protocol simply encapsulates the SCSI protocol into the
> > TCP/IP protocol, and carries packets over IP networks. You can handle
> ...
> 
> On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> > Actually, this is property not of iSCSI target itself, but of any SCSI target.
> > So, we implemented it as part of our SCSI target mid-level (SCST,
> > http://scst.sourceforge.net), therefore any target driver working over it will
> > automatically benefit from this feature. Unfortunately, currently available
> > only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
> > works not too reliable and only with very specific initiators). The published
> ...
> 
> The above confirms basically my understanding apart from one "minor" 
> confusion - I thought, that parallel to hardware solutions pure software 
> implementations were possible / being developed, like a driver, that 
> implements a SCSI LDD API on one side, and forwards packets to an IP 
> stack, say, over an ethernet card - on the initiator side. And a counter 
> part on the target side. Similarly to the USB mass-storage and storage 
> gadget drivers?
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: several messages

[Guennadi Liakhovetski]

Hello and thanks for the replies

On Wed, 11 May 2005, FUJITA Tomonori wrote:
> The iSCSI protocol simply encapsulates the SCSI protocol into the
> TCP/IP protocol, and carries packets over IP networks. You can handle
...

On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> Actually, this is property not of iSCSI target itself, but of any SCSI target.
> So, we implemented it as part of our SCSI target mid-level (SCST,
> http://scst.sourceforge.net), therefore any target driver working over it will
> automatically benefit from this feature. Unfortunately, currently available
> only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
> works not too reliable and only with very specific initiators). The published
...

The above confirms basically my understanding apart from one "minor" 
confusion - I thought, that parallel to hardware solutions pure software 
implementations were possible / being developed, like a driver, that 
implements a SCSI LDD API on one side, and forwards packets to an IP 
stack, say, over an ethernet card - on the initiator side. And a counter 
part on the target side. Similarly to the USB mass-storage and storage 
gadget drivers?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: several messages

[Bill Davidsen]

Thank all of you for this information. This is an interesting way to
overcome the kernel memory fragmentation issue. I would have thought it
was more important to address having the memory so fragmented that there
is no 8k chunk left "even with many megabytes free" as someone wrote.


On Mon, 10 May 2004, Horst von Brand wrote:

> Bill Davidsen <davidsen@tmr.com> said:
> 
> [...]
> 
> > I tried 4k stack, I couldn't measure any improvement in anything (as in 
> > no visible speedup or saving in memory).
> 
> 4K stacks lets the kernel create new threads/processes as long as there is
> free memory; with 8K stacks it needs two consecutive free page frames in
> physical memory, when memory is fragmented (and large) they are hard to
> come by...
> -- 
> Dr. Horst H. von Brand                   User #22616 counter.li.org
> Departamento de Informatica                     Fono: +56 32 654431
> Universidad Tecnica Federico Santa Maria              +56 32 654239
> Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513
> 

On Mon, 10 May 2004, Andrew Morton wrote:

> Horst von Brand <vonbrand@inf.utfsm.cl> wrote:
> >
> > Bill Davidsen <davidsen@tmr.com> said:
> > 
> > [...]
> > 
> > > I tried 4k stack, I couldn't measure any improvement in anything (as in 
> > > no visible speedup or saving in memory).
> > 
> > 4K stacks lets the kernel create new threads/processes as long as there is
> > free memory; with 8K stacks it needs two consecutive free page frames in
> > physical memory, when memory is fragmented (and large) they are hard to
> > come by...
> 
> This is true to a surprising extent.  A couple of weeks ago I observed my
> 256MB box freeing over 20MB of pages before it could successfully acquire a
> single 1-order page.
> 
> That was during an updatedb run.
> 
> And a 1-order GFP_NOFS allocation was actually livelocking, because
> !__GFP_FS allocations aren't allowed to enter dentry reclaim.  Which is why
> VFS caches are now forced to use 0-order allocations.
> 
> 

On Tue, 11 May 2004, Helge Hafting wrote:

> Bill Davidsen wrote:
> 
> > Arjan van de Ven wrote:
> >
> >>> It's probably a Bad Idea to push this to Linus before the vendors 
> >>> that have
> >>> significant market-impact issues (again - anybody other than NVidia 
> >>> here?)
> >>> have gotten their stuff cleaned up...
> >>
> >>
> >>
> >> Ok I don't want to start a flamewar but... Do we want to hold linux back
> >> until all binary only module vendors have caught up ??
> >
> >
> > My questions is, hold it back from what? Having the 4k option is fine, 
> > it's just eliminating the current default which I think is 
> > undesirable. I tried 4k stack, I couldn't measure any improvement in 
> > anything (as in no visible speedup or saving in memory). 
> 
> The memory saving is usually modest: 4k per thread. It might make a 
> difference for
> those with many thousands of threads.   I believe this is unswappable 
> memory,
> which is much more valuable than ordinary process memory.
> 
> More interesting is that it removes one way for fork() to fail. With 8k 
> stacks,
> the new process needs to allocate two consecutive pages for those 8k.  That
> might be impossible due to fragmentation, even if there are megabytes of 
> free
> memory. Such a problem usually only shows up after a long time.  Now we 
> only need to
> allocate a single page, which always works as long as there is any free 
> memory at all.
> 
> > For an embedded system, where space is tight and the code paths well 
> > known, sure, but I haven't been able to find or generate any objective 
> > improvement, other than some posts saying smaller is always better. 
> > Nothing slows a system down like a crash, even if it isn't followed by 
> > a restore from backup.
> 
> Consider the case when your server (web/mail/other) fails to fork, and then
> you can't login because that requires fork() too.  4k stacks remove this 
> scenario,
> and is a stability improvement.
> 
> Helge Hafting
> 

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: several messages

[Ingo Molnar]

On Tue, 22 Apr 2003, Rick Lindsley wrote:

> True.  I have a hunch (and it's only a hunch -- no hard data!) that two
> threads that are sharing the same data will do better if they can be
> located on a physical/sibling processor group.  For workloads where you
> really do have two distinct processes, or even threads but which are
> operating on wholly different portions of data or code, moving them to
> separate physical processors may be warranted.  The key is whether the
> work of one sibling is destroying the cache of another.

If two threads have a workload that wants to be co-scheduled then the SMP
scheduler will do damage to them anyway - independently of any HT
scheduling decisions. One solution for such specific cases is to use the
CPU-binding API to move those threads to the same physical CPU. If there's
some common class of applications where this is the common case, then we
could start thinking about automatic support for them.

	Ingo

Re: several messages

[Rick Lindsley]

    Have you done any tests with a threaded process running on a single CPU in
    the siblings? If they are sharing data and locks in the same cache it's
    not obvious (to me at least) that it would be faster in two CPUs having to
    do updates. That's a question, not an implication that it is significantly
    better in just one, a threaded program with only two threads is not as
    likely to be doing the same thing in both, perhaps.

True.  I have a hunch (and it's only a hunch -- no hard data!) that
two threads that are sharing the same data will do better if they can
be located on a physical/sibling processor group.  For workloads where
you really do have two distinct processes, or even threads but which are
operating on wholly different portions of data or code, moving them to
separate physical processors may be warranted.  The key is whether the
work of one sibling is destroying the cache of another.

Rick

Re: several messages

[Bill Davidsen]

On Tue, 22 Apr 2003, Dave Jones wrote:

> On Mon, Apr 21, 2003 at 03:13:31PM -0400, Ingo Molnar wrote:
> 
>  > +/*
>  > + * Is there a way to do this via Kconfig?
>  > + */
>  > +#if CONFIG_NR_SIBLINGS_2
>  > +# define CONFIG_NR_SIBLINGS 2
>  > +#elif CONFIG_NR_SIBLINGS_4
>  > +# define CONFIG_NR_SIBLINGS 4
>  > +#else
>  > +# define CONFIG_NR_SIBLINGS 0
>  > +#endif
>  > +
> 
> Maybe this would be better resolved at runtime ?
> With the above patch, you'd need three seperate kernel images
> to run optimally on a system in each of the cases.
> The 'vendor kernel' scenario here looks ugly to me.
> 
>  > +#if CONFIG_NR_SIBLINGS
>  > +# define CONFIG_SHARE_RUNQUEUE 1
>  > +#else
>  > +# define CONFIG_SHARE_RUNQUEUE 0
>  > +#endif
> 
> And why can't this just be a
> 
> 	if (ht_enabled)
> 		shared_runqueue = 1;
> 
> Dumping all this into the config system seems to be the
> wrong direction IMHO. The myriad of runtime knobs in the
> scheduler already is bad enough, without introducing
> compile time ones as well.

May I add my "I don't understand this, either" at this point? It seems
desirable to have this particular value determined at runtime. 

On Tue, 22 Apr 2003, Ingo Molnar wrote:

> 
> On Tue, 22 Apr 2003, Rick Lindsley wrote:

> > [...] Are we assuming that because both a physical processor and its
> > sibling are not idle, that it is better to move a task from the sibling
> > to a physical processor?  In other words, we are presuming that the case
> > where the task on the physical processor and the task(s) on the
> > sibling(s) are actually benefitting from the relationship is rare?
> 
> yes. This 'un-sharing' of contexts happens unconditionally, whenever we
> notice the situation. (ie. whenever a CPU goes completely idle and notices
> an overloaded physical CPU.) On the HT system i have i have measure this
> to be a beneficial move even for the most trivial things like infinite
> loop-counting.
> 
> the more per-logical-CPU cache a given SMT implementation has, the less
> beneficial this move becomes - in that case the system should rather be
> set up as a NUMA topology and scheduled via the NUMA scheduler.

Have you done any tests with a threaded process running on a single CPU in
the siblings? If they are sharing data and locks in the same cache it's
not obvious (to me at least) that it would be faster in two CPUs having to
do updates. That's a question, not an implication that it is significantly
better in just one, a threaded program with only two threads is not as
likely to be doing the same thing in both, perhaps.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: several messages

[David Woodhouse]

davidsen@tmr.com said:
>  You make my point for me, there is no magic, and when building a
> module it should require that the directory be specified by either a
> command line option (as noted below) or by being built as part of a
> source tree. There *is* no good default in that particular case.

/lib/modules/`uname -r`/build _is_ a good default for a module to build 
again. It is correct in more cases than a simple failure to do anything.

For _installing_, the correct place to install the built objects is surely
/lib/modules/$(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION) where the 
variables are obtained from the top-level Makefile in the kernel sources 
you built against.

You _default_ to building and installing for the current kernel, if it's
installed properly. But of course you should be permitted to override that 
on the command line.


> Related for sure, the point I was making was that there is no good
> default place to put modules built outside a kernel source tree (and
> probably also when built for multiple kernels).

I disagree. Modutils will look in only one place -- the /lib/modules/... 
directory corresponding to the kernel version for which you built each 
module. Each module, therefore, should go into the directory corresponding 
to the version of the kernel against which it was built.

Finding the appropriate _installation_ directory is trivial, surely? You 
can even find it from the 'kernel_version' stamp _inside_ the object file, 
without any other information?

--
dwmw2

Re: several messages

[Bill Davidsen]

On Wed, 22 Jan 2003, David Woodhouse wrote:

> 
> davidsen@tmr.com said:
> >  `uname -r` is the kernel version of the running kernel. It is NOT by
> > magic the kernel version of the kernel you are building... 
> 
> Er, yes. And what's your point? 
> 
> There is _no_ magic that will find the kernel you want to build against
> today without any input from you. Using the build tree for the
> currently-running kernel, if installed in the standard place, is as good a
> default as any. Of course you should be permitted to override that default.

You make my point for me, there is no magic, and when building a module it
should require that the directory be specified by either a command line
option (as noted below) or by being built as part of a source tree. There
*is* no good default in that particular case.


On Wed, 22 Jan 2003, Hal Duston wrote:

> I use "INSTALL_MOD_PATH=put/the/modules/here/instead/of/lib/modules" in my
> .profile or whatever in order to drop the modules into another directory
> at "make modules_install" time.  Is this one of the things folks are
> talking about?

Related for sure, the point I was making was that there is no good default
place to put modules built outside a kernel source tree (and probably also
when built for multiple kernels). I was suggesting that the module tree of
the running kernel might be a really poor choice. I don't think I was
clear in my first post, I was not suggesting a better default, I was
suggesting that any default is likely to bite.

I'm not unhappy that Mr. Woodhouse disagrees, I just think he missed my
point the first time and I'm trying to clarify.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.