On Fri, Jan 22, 2016 at 01:17:40PM -0500, Mike Marshall wrote: > >> Objections, comments? > > I have no objections so far to your suggestions, I'm trying to keep > my co-workers looking in on this discussion... most other days > we're all lined up in adjacent cubes... FWIW, I do see a problem there. This "submitter has given up" flag would need to be cleared on retries ;-/ When can we end up retrying? We'd need * wait_for_cancellation_downcall/wait_for_matching_downcall finished * op_state_serviced(op) false * op->downcall.status set to -EAGAIN wait_for_matching_downcall() either returns with op_state_serviced() or returns one -ETIMEDOUT, -EINTR, -EAGAIN or -EIO, with its return value ending up in op->downcall.status. So if it was wait_for_matching_downcall(), we'd have to have returned -EAGAIN. Which can happen only with op_state_purged(op), i.e. with character device having been closed after the sucker had been placed on the lists... wait_for_cancellation_downcall() similar, but it never returns -EAGAIN. IOW, for it retries are irrelevant. Damnit, what's the point of picking a purged request from the request list in orangefs_devreq_read()? Ditto for orangefs_devreq_write_iter() and request hash... Note that purge *can't* happen during either of those - ->release() can't be called while something is in ->read() or ->write_iter(). So if we would ignore purged requests in those, the problem with retries clearing the "I gave up" flag wouldn't exist... It's a race anyway - submitter had been woken up when we'd been marking the request as purged, and it's going to remove the sucker from whatever list it's on very soon after getting woken up. Do you see any problems with skipping such request list/hash elements in orangefs_devreq_read()/orangefs_devreq_write_iter() resp.? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
