xiaoxiang781216 commented on code in PR #19596:
URL: https://github.com/apache/nuttx/pull/19596#discussion_r3699623590
##########
drivers/sensors/sensor.c:
##########
@@ -1184,20 +1177,12 @@ static int sensor_poll(FAR struct file *filep,
fds->priv = filep;
if (lower->ops->fetch)
{
- /* Always return POLLIN for fetch data directly(non-block) */
+ /* Always return POLLIN for fetch only sensor: the data is read
+ * from the device on demand by sensor_read(), so there is never
+ * anything to wait for.
+ */
- if (filep->f_oflags & O_NONBLOCK)
- {
- eventset |= POLLIN;
- }
- else
- {
- nxsem_get_value(&user->buffersem, &semcount);
Review Comment:
should we remove buffersem too?
##########
drivers/sensors/sensor.c:
##########
@@ -96,6 +97,17 @@ struct sensor_user_s
struct list_node node; /* Node of users list */
struct pollfd *fds; /* The poll structure of thread waiting events
*/
sensor_role_t role; /* The is used to indicate user's role based on
open flags */
+#ifdef CONFIG_SCHED_LPWORK
+
+ /* Paces POLLIN for a fetch() only lower half at the interval this user
+ * asked for: upper is the device it belongs to, and fetched is when
+ * POLLIN was last reported to it, in microseconds.
+ */
+
+ FAR struct sensor_upperhalf_s *upper;
+ struct work_s work;
Review Comment:
let's change to wdog
##########
drivers/sensors/sensor.c:
##########
@@ -1204,6 +1303,19 @@ static int sensor_poll(FAR struct file *filep,
errout:
nxrmutex_unlock(&upper->lock);
+
+#ifdef CONFIG_SCHED_LPWORK
+ /* Cancel outside the lock: the worker takes it, so cancelling while
+ * holding it would deadlock. fds was cleared above, so a worker that is
+ * already running will neither notify nor re-arm.
+ */
+
+ if (!setup)
+ {
+ work_cancel_sync(LPWORK, &user->work);
Review Comment:
move after line 1301
##########
drivers/sensors/sensor.c:
##########
@@ -809,6 +867,19 @@ static int sensor_close(FAR struct file *filep)
sensor_update_latency(filep, upper, user, UINT32_MAX);
sensor_update_nonwakeup(filep, upper, user, true);
+#ifdef CONFIG_SCHED_LPWORK
+ /* A close() racing an armed poll() would otherwise leave the worker
+ * queued against the user we are about to free. Clear fds first so a
+ * worker already running cannot re-arm itself past the cancel, then
+ * cancel outside the lock, since the worker takes it too.
+ */
+
+ nxrmutex_lock(&upper->lock);
+ user->fds = NULL;
Review Comment:
why need? before close, sensor_poll with false should be called first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
