Re: [PATCH 1/1] console: add /proc/consoles

Re: [PATCH 1/1] console: add /proc/consoles

[Greg KH]

On Wed, Nov 03, 2010 at 04:35:09PM +0100, Jiri Slaby wrote:
> It allows users to see what consoles are currently known to the system
> and with what flags.
> 
> It is based on Werner's patch, the part about traversing fds was
> removed, the code was moved to kernel/printk.c, where consoles are
> handled and it makes more sense to me.

Why kernel/printk.c?  I don't think that makes sense, it's just a random
proc file, so why not put it into something like fs/proc/ instead?

Does it rely on any functions in the printk.c file?

thanks,

greg k-h

Re: [PATCH 1/1] console: add /proc/consoles

[Jiri Slaby]

On 11/03/2010 05:13 PM, Greg KH wrote:
> On Wed, Nov 03, 2010 at 04:35:09PM +0100, Jiri Slaby wrote:
>> It allows users to see what consoles are currently known to the system
>> and with what flags.
>>
>> It is based on Werner's patch, the part about traversing fds was
>> removed, the code was moved to kernel/printk.c, where consoles are
>> handled and it makes more sense to me.
> 
> Why kernel/printk.c?  I don't think that makes sense, it's just a random
> proc file, so why not put it into something like fs/proc/ instead?
> 
> Does it rely on any functions in the printk.c file?

No it doesn't. I will move it to fs/proc/ if that's preferred. I checked
how VM proc stuff is handled and it was in in mm/, so I put this into
kernel/...

(Then it will depend on the console cleanup series which I sent few
minutes ago...)

thanks,
-- 
js
suse labs

Re: [PATCH 1/1] console: add /proc/consoles

[Greg KH]

On Wed, Nov 03, 2010 at 05:16:11PM +0100, Jiri Slaby wrote:
> On 11/03/2010 05:13 PM, Greg KH wrote:
> > On Wed, Nov 03, 2010 at 04:35:09PM +0100, Jiri Slaby wrote:
> >> It allows users to see what consoles are currently known to the system
> >> and with what flags.
> >>
> >> It is based on Werner's patch, the part about traversing fds was
> >> removed, the code was moved to kernel/printk.c, where consoles are
> >> handled and it makes more sense to me.
> > 
> > Why kernel/printk.c?  I don't think that makes sense, it's just a random
> > proc file, so why not put it into something like fs/proc/ instead?
> > 
> > Does it rely on any functions in the printk.c file?
> 
> No it doesn't. I will move it to fs/proc/ if that's preferred. I checked
> how VM proc stuff is handled and it was in in mm/, so I put this into
> kernel/...
> 
> (Then it will depend on the console cleanup series which I sent few
> minutes ago...)

That's fine, I can take those through my tree as well, as it makes sense
to do so.

thanks,

greg k-h

Re: [PATCH 1/1] console: add /proc/consoles

[Jiri Slaby]

On 11/03/2010 05:22 PM, Greg KH wrote:
> On Wed, Nov 03, 2010 at 05:16:11PM +0100, Jiri Slaby wrote:
>> On 11/03/2010 05:13 PM, Greg KH wrote:
>>> On Wed, Nov 03, 2010 at 04:35:09PM +0100, Jiri Slaby wrote:
>>>> It allows users to see what consoles are currently known to the system
>>>> and with what flags.
>>>>
>>>> It is based on Werner's patch, the part about traversing fds was
>>>> removed, the code was moved to kernel/printk.c, where consoles are
>>>> handled and it makes more sense to me.
>>>
>>> Why kernel/printk.c?  I don't think that makes sense, it's just a random
>>> proc file, so why not put it into something like fs/proc/ instead?
>>>
>>> Does it rely on any functions in the printk.c file?
>>
>> No it doesn't. I will move it to fs/proc/ if that's preferred. I checked
>> how VM proc stuff is handled and it was in in mm/, so I put this into
>> kernel/...
>>
>> (Then it will depend on the console cleanup series which I sent few
>> minutes ago...)
> 
> That's fine, I can take those through my tree as well, as it makes sense
> to do so.

Actually where this code should be in fs/proc/? Most of the /proc/* is
handled elsewhere (fs/ mm/ kernel/). The rest is handled in specialized
fs/proc/FILE.c.

I cannot rule out a file to put this in.

thanks,
-- 
js
suse labs

Re: [PATCH 1/1] console: add /proc/consoles

[Greg KH]

On Wed, Nov 03, 2010 at 05:25:32PM +0100, Jiri Slaby wrote:
> On 11/03/2010 05:22 PM, Greg KH wrote:
> > On Wed, Nov 03, 2010 at 05:16:11PM +0100, Jiri Slaby wrote:
> >> On 11/03/2010 05:13 PM, Greg KH wrote:
> >>> On Wed, Nov 03, 2010 at 04:35:09PM +0100, Jiri Slaby wrote:
> >>>> It allows users to see what consoles are currently known to the system
> >>>> and with what flags.
> >>>>
> >>>> It is based on Werner's patch, the part about traversing fds was
> >>>> removed, the code was moved to kernel/printk.c, where consoles are
> >>>> handled and it makes more sense to me.
> >>>
> >>> Why kernel/printk.c?  I don't think that makes sense, it's just a random
> >>> proc file, so why not put it into something like fs/proc/ instead?
> >>>
> >>> Does it rely on any functions in the printk.c file?
> >>
> >> No it doesn't. I will move it to fs/proc/ if that's preferred. I checked
> >> how VM proc stuff is handled and it was in in mm/, so I put this into
> >> kernel/...
> >>
> >> (Then it will depend on the console cleanup series which I sent few
> >> minutes ago...)
> > 
> > That's fine, I can take those through my tree as well, as it makes sense
> > to do so.
> 
> Actually where this code should be in fs/proc/? Most of the /proc/* is
> handled elsewhere (fs/ mm/ kernel/). The rest is handled in specialized
> fs/proc/FILE.c.

What's wrong with putting it into fs/proc/proc_tty.c?  That seems like
the most logical thing to me...

thanks,

greg k-h

Re: [PATCH 1/1] console: add /proc/consoles

[Jiri Slaby]

On 11/03/2010 06:39 PM, Greg KH wrote:
> On Wed, Nov 03, 2010 at 05:25:32PM +0100, Jiri Slaby wrote:
>> Actually where this code should be in fs/proc/? Most of the /proc/* is
>> handled elsewhere (fs/ mm/ kernel/). The rest is handled in specialized
>> fs/proc/FILE.c.
> 
> What's wrong with putting it into fs/proc/proc_tty.c?  That seems like
> the most logical thing to me...

consoles have little to do with ttys. Except ttys can be consoles. To me
it seems illogical to have info about netconsole, parallel consoles or
early serials somewhere in /proc/tty/. That's why I moved the info from
/proc/tty/ to /proc and the code from fs/proc/proc_tty.c to
kernel/printk.c where consoles are managed.

But if that makes less sense now than it did before, I will happily
switch it back.

regards,
-- 
js

Re: [PATCH 1/1] console: add /proc/consoles

[Greg KH]

On Wed, Nov 03, 2010 at 09:03:42PM +0100, Jiri Slaby wrote:
> On 11/03/2010 06:39 PM, Greg KH wrote:
> > On Wed, Nov 03, 2010 at 05:25:32PM +0100, Jiri Slaby wrote:
> >> Actually where this code should be in fs/proc/? Most of the /proc/* is
> >> handled elsewhere (fs/ mm/ kernel/). The rest is handled in specialized
> >> fs/proc/FILE.c.
> > 
> > What's wrong with putting it into fs/proc/proc_tty.c?  That seems like
> > the most logical thing to me...
> 
> consoles have little to do with ttys. Except ttys can be consoles.

And all consoles are ttys :)

> To me
> it seems illogical to have info about netconsole, parallel consoles or
> early serials somewhere in /proc/tty/. That's why I moved the info from
> /proc/tty/ to /proc and the code from fs/proc/proc_tty.c to
> kernel/printk.c where consoles are managed.

Ok, how about fs/proc/proc_console.c instead?

And no, printk.c is not the most logical place for something like this.
If I see a proc file, I am not going to automatically assume that
printk.c would have anything to do with it.  As it really doesn't
(printk goes to the kernel log buffer, not a console).

thanks,

greg k-h

Re: [PATCH 1/1] console: add /proc/consoles

[Jiri Slaby]

On 11/03/2010 10:12 PM, Greg KH wrote:
> On Wed, Nov 03, 2010 at 09:03:42PM +0100, Jiri Slaby wrote:
>> On 11/03/2010 06:39 PM, Greg KH wrote:
>>> On Wed, Nov 03, 2010 at 05:25:32PM +0100, Jiri Slaby wrote:
>>>> Actually where this code should be in fs/proc/? Most of the /proc/* is
>>>> handled elsewhere (fs/ mm/ kernel/). The rest is handled in specialized
>>>> fs/proc/FILE.c.
>>>
>>> What's wrong with putting it into fs/proc/proc_tty.c?  That seems like
>>> the most logical thing to me...
>>
>> consoles have little to do with ttys. Except ttys can be consoles.
> 
> And all consoles are ttys :)

Oh, they are not. Or maybe I'm missing something?

>> To me
>> it seems illogical to have info about netconsole, parallel consoles or
>> early serials somewhere in /proc/tty/. That's why I moved the info from
>> /proc/tty/ to /proc and the code from fs/proc/proc_tty.c to
>> kernel/printk.c where consoles are managed.
> 
> Ok, how about fs/proc/proc_console.c instead?

Yes, this sounds good.

> And no, printk.c is not the most logical place for something like this.
> If I see a proc file, I am not going to automatically assume that
> printk.c would have anything to do with it.  As it really doesn't
> (printk goes to the kernel log buffer, not a console).

Actually it does all the job:
* filling the log/ring buffer (v/printk)
* managing consoles (un/register_console)
* writing to consoles (call_console_drivers and friends)

thanks,
-- 
js

Re: [PATCH 1/1] console: add /proc/consoles

[Greg KH]

On Thu, Nov 04, 2010 at 10:57:02AM +0100, Jiri Slaby wrote:
> On 11/03/2010 10:12 PM, Greg KH wrote:
> > On Wed, Nov 03, 2010 at 09:03:42PM +0100, Jiri Slaby wrote:
> >> On 11/03/2010 06:39 PM, Greg KH wrote:
> >>> On Wed, Nov 03, 2010 at 05:25:32PM +0100, Jiri Slaby wrote:
> >>>> Actually where this code should be in fs/proc/? Most of the /proc/* is
> >>>> handled elsewhere (fs/ mm/ kernel/). The rest is handled in specialized
> >>>> fs/proc/FILE.c.
> >>>
> >>> What's wrong with putting it into fs/proc/proc_tty.c?  That seems like
> >>> the most logical thing to me...
> >>
> >> consoles have little to do with ttys. Except ttys can be consoles.
> > 
> > And all consoles are ttys :)
> 
> Oh, they are not. Or maybe I'm missing something?

No, you are correct, I was wrong.

> >> To me
> >> it seems illogical to have info about netconsole, parallel consoles or
> >> early serials somewhere in /proc/tty/. That's why I moved the info from
> >> /proc/tty/ to /proc and the code from fs/proc/proc_tty.c to
> >> kernel/printk.c where consoles are managed.
> > 
> > Ok, how about fs/proc/proc_console.c instead?
> 
> Yes, this sounds good.

Great.

thanks,

greg k-h