Re: [PATCH 2/3] ptp: support multiple timestamp event readers
From: Simon Horman <horms@kernel.org>
Date: 2023-09-06 18:13:53
On Wed, Sep 06, 2023 at 12:47:53PM +0200, Xabier Marquiegui wrote:
Use linked lists to create one event queue per open file. This enables simultaneous readers for timestamp event queues. Signed-off-by: Xabier Marquiegui <redacted>
Hi Xabier, some minor feedback from my side ...
quoted hunk ↗ jump to hunk
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index 1ea11f864abb..c65dc6fefaa6 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c@@ -103,9 +103,39 @@ int ptp_set_pinfunc(struct ptp_clock *ptp, unsigned int pin, int ptp_open(struct posix_clock *pc, fmode_t fmode) { + struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock); + struct timestamp_event_queue *queue; + + queue = kzalloc(sizeof(struct timestamp_event_queue), GFP_KERNEL); + if (queue == NULL)
nit: this could be: if (!queue)
+ return -EINVAL; + queue->reader_pid = task_pid_nr(current); + list_add_tail(&queue->qlist, &ptp->tsevqs); + return 0; }
...
quoted hunk ↗ jump to hunk
@@ -436,14 +466,25 @@ __poll_t ptp_poll(struct posix_clock *pc, struct file *fp, poll_table *wait) { struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock); struct timestamp_event_queue *queue; + struct list_head *pos, *n; + bool found = false; + pid_t reader_pid = task_pid_nr(current); poll_wait(fp, &ptp->tsev_wq, wait); /* - * Extract only the first element in the queue list - * TODO: Identify the relevant queue + * Extract only the desired element in the queue list */ - queue = list_entry(&ptp->tsevqs, struct timestamp_event_queue, qlist); + list_for_each_safe(pos, n, &ptp->tsevqs) { + queue = list_entry(pos, struct timestamp_event_queue, qlist); + if (queue->reader_pid == reader_pid) { + found = true; + break; + } + } + + if (!found) + return -EINVAL;
Sparse complains that the return type for this function is __poll_t, but here an int is returned. Perhaps returning EPOLLERR is appropriate here?
return queue_cnt(queue) ? EPOLLIN : 0; }
...