lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 21 Sep 2010 08:39:05 +0200 From: Florian Tobias Schandinat <FlorianSchandinat@....de> To: Bruno Prémont <bonbons@...ux-vserver.org> CC: linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org, Bernie Thompson <bernie@...gable.com> Subject: Re: [Patch, RFC] Make struct fb_info ref-counted with kref Bruno Prémont schrieb: > On Tue, 21 Sep 2010 00:28:27 Florian Tobias Schandinat <FlorianSchandinat@....de> wrote: >>> Tracking if/how often framebuffer is opened as such is a separate thing (though >>> all users that have the framebuffer opened hold a reference to fb_info). >> That's what I said. So as long as refcount <= 1 it does not matter whether you >> just count on open/release or additionally on every framebuffer operation, just >> that the later produces more noise. > > Hm, I don't count on every framebuffer operation... in most cases > fb_info is provided as function argument, in which case no further > counting is needed as the caller has a valid reference. > > With my patch applied refcount for registered but unsed framebuffer was > 2 (once for the driver, once for registered_fb entry) and went up to 3 > when userspace opened framebuffer. fbcon's usage only incremented > refcount for very short timeframes when effectively using fb_info. > > When starting with the FB minor I have to take a new reference. > (though I maybe should check if file's private data is set and use > that reference instead of looking up fb_info by minor as is currently > done) > > For fbcon all the references are taken by FB minor (I wondered why > fbcon only remembers index into registered_fb aka minor instead of > fb_info itself) True, I guess fb infrastructure and fbcon both could use a lot of work. At the moment I am more at fixing my driver but once that's done to an acceptable level I think I'll give it a try, too. >> So I still don't see any advantage in counting users + uses. >> Please note that I do not object the idea of the patch itself, it's only that I >> have a different preference on what to count. I only want to express that your >> way is more complicated than what I would recommend. > > I don't think I see how you would do the refcounting... would you just > drop the changes in fb_open() and fb_release()? > Could you describe your approach (with pseudo-code) or the differences > to mine? No, quite the opposite. I would increase the refcount in fb_open in fbmem.c and in fbcon.c where fbops->fb_open is called; decrease the refcount on fb_release in fbmem.c and in fbcon.c where fbops->fb_release is called. This would be only 6 places in total which need to be changed and would be the same as what you said is currently used. As we agree (I hope) that for framebuffer operations an already open framebuffer is required this would not change when the framebuffer will be freed (compared to your counting) so the part changing how a framebuffer is shut down can be changed just as you proposed. The advantage is that it does not require changes to framebuffer ioctls/read/write/mmap and probably also much less changes in fbcon. And if this is also what drivers want and there is no conflict with what you want, I don't see any reason to not provide this service but force them to do this kind of refcounting on their own. Thanks, Florian Tobias Schandinat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists