linux-kernel - Re: [PATCH v8 17/22] counter: Add character device interface

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 Feb 2021 10:53:07 +0900
From:   William Breathitt Gray <vilhelm.gray@...il.com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     jic23@...nel.org, kamel.bouhara@...tlin.com, gwendal@...omium.org,
        a.fatoum@...gutronix.de, david@...hnology.com,
        linux-iio@...r.kernel.org, patrick.havelange@...ensium.com,
        alexandre.belloni@...tlin.com, mcoquelin.stm32@...il.com,
        linux-kernel@...r.kernel.org,
        Dan Carpenter <dan.carpenter@...cle.com>,
        kernel@...gutronix.de, fabrice.gasnier@...com,
        syednwaris@...il.com, linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, alexandre.torgue@...com
Subject: Re: [PATCH v8 17/22] counter: Add character device interface

On Mon, Feb 15, 2021 at 10:24:12AM +0100, Oleksij Rempel wrote:
> Hi William,
> 
> On Fri, Feb 12, 2021 at 09:13:41PM +0900, William Breathitt Gray wrote:
> > This patch introduces a character device interface for the Counter
> > subsystem. Device data is exposed through standard character device read
> > operations. Device data is gathered when a Counter event is pushed by
> > the respective Counter device driver. Configuration is handled via ioctl
> > operations on the respective Counter character device node.
> 
> .....
> 
> > +/**
> > + * counter_push_event - queue event for userspace reading
> > + * @counter:	pointer to Counter structure
> > + * @event:	triggered event
> > + * @channel:	event channel
> > + *
> > + * Note: If no one is watching for the respective event, it is silently
> > + * discarded.
> > + */
> > +void counter_push_event(struct counter_device *const counter, const u8 event,
> > +			const u8 channel)
> > +{
> > +	struct counter_event ev = {0};
> > +	unsigned int copied = 0;
> > +	unsigned long flags;
> > +	struct counter_event_node *event_node;
> > +	struct counter_comp_node *comp_node;
> > +
> > +	ev.timestamp = ktime_get_ns();
> > +	ev.watch.event = event;
> > +	ev.watch.channel = channel;
> > +
> > +	/* Could be in an interrupt context, so use a raw spin lock */
> > +	raw_spin_lock_irqsave(&counter->events_list_lock, flags);
> > +
> > +	/* Search for event in the list */
> > +	list_for_each_entry(event_node, &counter->events_list, l)
> > +		if (event_node->event == event &&
> > +		    event_node->channel == channel)
> > +			break;
> > +
> > +	/* If event is not in the list */
> > +	if (&event_node->l == &counter->events_list)
> > +		goto exit_early;
> 
> To cover my "interrupt-counter" use case, I added following line here to
> convert the tail drop fifo to the head drop fifo:
> 
> +       list_for_each_entry(comp_node, &event_node->comp_list, l)
> +               to_copy += sizeof(ev);
> +
> +       while (kfifo_avail(&counter->events) < to_copy)
> +               kfifo_skip(&counter->events);
> 
> May be it make sense to make it optional and integrate in to you patches
> before it goes mainline?
> 
> Regards,
> Oleksij
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Hello Oleksij,

I appreciate your suggestion, and I'm excited about your
interrupt-counter driver -- it looks like the Counter character device
interface will be central for that driver. However, I want to hold off
on adding further functionality to this patchset so that we can stablize
it enough to merge.

Once the Counter character device interface is merged, please send your
changes again as a patch and we can review it further then.

Thank you,

William Breathitt Gray

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)