Re: develoment workflow: how to avoid duplicate work ?

2018-06-05 Thread Hugo Lefeuvre
> > It's not duplication, it's increment/decrement of a counter.
> > 
> > Look for functions  with 'get' and 'put' in their names.
> 
> AFAICT counter is f_count in struct file, updated by fget and fput.

Yes, this is the counter I was speaking about. But to me the only way to
increment/decrement this counter was either to duplicate an existing
reference using dup/dup2, or to fork the process. This was wrong, and
I'm glad I learned that.

FTR: After taking a look at the kernel code, the ioctl syscall is
implemented in ksys_ioctl[0], which calls fdget/fdput to get the struct
fd, incrementing and decrementing the count.

Thanks your help !

> > I haven't looked at the code - there's an outside chance that the driver
> > isn't doing reference counting correctly.
> 
> pi433 driver does not mess with struct file too much so I guess it should
> be safe :)

I'll take a closer look at the TODO and come back later with a patch. If
there's nothing to do I'll remove it, otherwise I'll fix it.

Cheers,
 Hugo

[0] https://elixir.bootlin.com/linux/latest/source/fs/ioctl.c#L692

-- 
 Hugo Lefeuvre (hle)|www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: a Linux device driver

2018-06-05 Thread valdis . kletnieks
On Tue, 05 Jun 2018 13:51:54 -0700, Abu Rasheda said:

> right now, I am in a mood to write a driver,  not testing :)

Can you, in a few sentences, explain why the Linux community wants a driver
written by somebody who wanted to write a driver but didn't want to do the
testing needed to ensure it doesn't blow up the card, or burn out the USB hub,
or turn your dog green?



pgpMiYy__eeoh.pgp
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: a Linux device driver

2018-06-05 Thread Abu Rasheda
On Tue, Jun 5, 2018 at 1:51 AM, Ozgur Kara  wrote:

>
>
> 05.06.2018, 08:57, "valdis.kletni...@vt.edu" :
> > On Mon, 04 Jun 2018 15:49:37 -0700, Abu Rasheda said:
> >
> >>  Any recommendations what device driver is missing and open source
> >>  community could benefit from a new driver or enhance some existing
> driver?
> >
> > Short answer: One that you have the hardware for so you can actually
> test your code.
> > Nobody wants to see a driver for a Frobnozz 9000 graphics card written
> by somebody who
> > doesn't have access to a Frobmozz 9000 card for testing.
> >
>

wondering which hardware is missing a driver, I can try yo buy/acquire the
hardware

> Long answer: https://lists.kernelnewbies.org/pipermail/kernelnewbies/
> 2017-April/017765.html
>

right now, I am in a mood to write a driver,  not testing :)


> in addition,
>
> You can browse the staging directory in the kernel source and check.
>
>
will do thanks

Regards
>
> Ozgur
>
> >
> > ___
> > Kernelnewbies mailing list
> > Kernelnewbies@kernelnewbies.org
> > https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: develoment workflow: how to avoid duplicate work ?

2018-06-05 Thread valdis . kletnieks
On Tue, 05 Jun 2018 10:20:16 -0400, Hugo Lefeuvre said:

> Thanks. I think I'll have to read the source code to fully understand
> what happens. Do you know what piece of code handles this reference
> duplication ?

It's not duplication, it's increment/decrement of a counter.

Look for functions  with 'get' and 'put' in their names.

> As a conclusion we can assume that ioctl and release never run
> concurrently, and as such the lock introduced in my patch is useless.

Well, they *shouldn't* do so.  What the code actually does, however

> Concerning the TODO at line 876, I think I've misunderstood it. I'll
> think a bit more about it and come back with an updated patch later.

I haven't looked at the code - there's an outside chance that the driver
isn't doing reference counting correctly.


pgpKD7Q7Lgaa1.pgp
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: develoment workflow: how to avoid duplicate work ?

2018-06-05 Thread Hugo Lefeuvre
> > Do you mean that the ioctl/read/write call increments the reference
> > count in this case ? It would mean that these syscalls aren't really
> > using passed FD but instead create duplicates to make sure the open
> > file description won't be freed during their execution, right ?
> 
> One file descriptor is passed around, and each syscall or other code that 
> needs
> to protect it from evaporating out from under it takes a reference.

Thanks. I think I'll have to read the source code to fully understand
what happens. Do you know what piece of code handles this reference
duplication ?

As a conclusion we can assume that ioctl and release never run
concurrently, and as such the lock introduced in my patch is useless.

Concerning the TODO at line 876, I think I've misunderstood it. I'll
think a bit more about it and come back with an updated patch later.

Thanks !

Cheers,
 Hugo

-- 
 Hugo Lefeuvre (hle)|www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA


signature.asc
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: a Linux device driver

2018-06-05 Thread Ozgur Kara


05.06.2018, 08:57, "valdis.kletni...@vt.edu" :
> On Mon, 04 Jun 2018 15:49:37 -0700, Abu Rasheda said:
>
>>  Any recommendations what device driver is missing and open source
>>  community could benefit from a new driver or enhance some existing driver?
>
> Short answer: One that you have the hardware for so you can actually test 
> your code.
> Nobody wants to see a driver for a Frobnozz 9000 graphics card written by 
> somebody who
> doesn't have access to a Frobmozz 9000 card for testing.
>
> Long answer: 
> https://lists.kernelnewbies.org/pipermail/kernelnewbies/2017-April/017765.html

in addition,

You can browse the staging directory in the kernel source and check.

Regards

Ozgur

>
> ___
> Kernelnewbies mailing list
> Kernelnewbies@kernelnewbies.org
> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies