remove name length check in a workqueue

Ingo,

This has been in the workqueue code in day one, for no real reason that I can see. We just tripped over it in SCSI because the fibre channel transport class creates one workqueue per host with the name scsi_wq_%d which trips this after we get to 100. Unfortunately we just came across someone with > 100 host adapters ...

I think the solution is just to get rid of the artificial limit.

James

kernel/workqueue.c +0 -2

@@ -308,8 +308,6 @@ struct workqueue_struct *__create_workqu
 	struct workqueue_struct *wq;
 	struct task_struct *p;
 
-	BUG_ON(strlen(name) > 10);
-
 	wq = kmalloc(sizeof(*wq), GFP_KERNEL);
 	if (!wq)
 		return NULL;

yeah ... cannot remember why i have done it originally :-|

Quoted 27 lines

Ingo,

This has been in the workqueue code in day one, for no real reason that I can see. We just tripped over it in SCSI because the fibre channel transport class creates one workqueue per host with the name scsi_wq_%d which trips this after we get to 100. Unfortunately we just came across someone with > 100 host adapters ...

I think the solution is just to get rid of the artificial limit.

James

kernel/workqueue.c +0 -2

@@ -308,8 +308,6 @@ struct workqueue_struct *__create_workqu
 	struct workqueue_struct *wq;
 	struct task_struct *p;
 
-	BUG_ON(strlen(name) > 10);
-
 	wq = kmalloc(sizeof(*wq), GFP_KERNEL);
 	if (!wq)
 		return NULL;

Quoted 1 line · Ingo Molnar

yeah ... cannot remember why i have done it originally :-|

Might it be to do with sizeof(task_struct.comm)?

Quoted 4 lines · Andrew Morton

Quoted 1 line · Ingo Molnar

yeah ... cannot remember why i have done it originally :-|

Might it be to do with sizeof(task_struct.comm)?

But that's 16 bytes not 10;

and anyway, it doesn't have to be unique;
set_task_comm just does a strlcpy from the name, so it will be truncated (same as for a binary with > 15 character name).

Quoted 3 lines

and anyway, it doesn't have to be unique;
set_task_comm just does a strlcpy from the name, so it will be truncated (same as for a binary with > 15 character name).

Yup. But it'd be fairly silly to go adding the /%d, only to have it truncated off again.

We could truncate the name before adding the CPU number, but it sounds saner to just prevent anyone passing in excessively long names. Via BUG_ON, say ;)

What's the actual problem?

Quoted 4 lines · Andrew Morton

Quoted 3 lines

and anyway, it doesn't have to be unique;
set_task_comm just does a strlcpy from the name, so it will be truncated (same as for a binary with > 15 character name).

Quoted 2 lines

Yup. But it'd be fairly silly to go adding the /%d, only to have it truncated off again.

Well, but the other alternative is that we hit arbitrary BUG_ON() limits in systems that create numbered workqueues which is rather contrary to our scaleability objectives, isn't it?

Quoted 1 line

What's the actual problem?

What I posted originally; the current SCSI format for a workqueue:
scsi_wq_%d hits the bug after the host number rises to 100, which has been seen by some enterprise person with > 100 HBAs.

The reason for this name is that the error handler thread is called scsi_eh_%d; so we could rename all our threads to avoid this, but one day someone will come along with a huge enough machine to hit whatever limit we squeeze it down to.

Quoted 8 lines

What I posted originally; the current SCSI format for a workqueue:
scsi_wq_%d hits the bug after the host number rises to 100, which has been seen by some enterprise person with > 100 HBAs.

The reason for this name is that the error handler thread is called scsi_eh_%d; so we could rename all our threads to avoid this, but one day someone will come along with a huge enough machine to hit whatever limit we squeeze it down to.

OK, well scsi is using single-threaded workqueues anyway. So we could do:

	if (singlethread)
		BUG_ON(strlen(name) > sizeof(task_struct.comm) - 1);
	else
		BUG_ON(strlen(name) > sizeof(task_struct.comm) - 1 - 4);

which gets you 10,000,000 HBAs. Enough?

Quoted 6 lines · James Bottomley

Quoted 4 lines · Andrew Morton

Quoted 3 lines

and anyway, it doesn't have to be unique;
set_task_comm just does a strlcpy from the name, so it will be truncated (same as for a binary with > 15 character name).

Quoted 6 lines

Quoted 2 lines

Yup. But it'd be fairly silly to go adding the /%d, only to have it truncated off again.

Well, but the other alternative is that we hit arbitrary BUG_ON() limits in systems that create numbered workqueues which is rather contrary to our scaleability objectives, isn't it?

Another alternative is to stop passing in such long strings ;)

Quoted 10 lines

Quoted 1 line

What's the actual problem?

What I posted originally; the current SCSI format for a workqueue:
scsi_wq_%d hits the bug after the host number rises to 100, which has been seen by some enterprise person with > 100 HBAs.

The reason for this name is that the error handler thread is called scsi_eh_%d; so we could rename all our threads to avoid this, but one day someone will come along with a huge enough machine to hit whatever limit we squeeze it down to.

OK, well scsi is using single-threaded workqueues anyway. So we could do:

        if (singlethread)
                BUG_ON(strlen(name) > sizeof(task_struct.comm) - 1);
        else
                BUG_ON(strlen(name) > sizeof(task_struct.comm) - 1 - 4);

which gets you 10,000,000 HBAs. Enough?

Ho hum, OK, let's just kill the BUG_ON.

Well, but the other alternative is that we hit arbitrary BUG_ON() limits in systems that create numbered workqueues which is rather contrary to our scaleability objectives, isn't it?

I think I'd rather the name truncation than have to respond to kernel BUG()'s. If someone really has a problem with the appearance of ps, they can always increase TASK_COMM_LEN.

Quoted 7 lines

We could truncate the name before adding the CPU number, but it sounds saner to just prevent anyone passing in excessively long names. Via BUG_ON, say ;)

What's the actual problem?

What I posted originally; the current SCSI format for a workqueue:
scsi_wq_%d hits the bug after the host number rises to 100, which has been seen by some enterprise person with > 100 HBAs.

The reason for this name is that the error handler thread is called scsi_eh_%d; so we could rename all our threads to avoid this, but one day someone will come along with a huge enough machine to hit whatever limit we squeeze it down to.

James