Question about retry logic in usb-skeleton.c skel_read()

[const]

Hello,

I'm a computer science undergraduate student studying Linux kernel USB device drivers.

While analyzing the skel_read() function in usb-skeleton.c, I noticed what seems like inconsistent retry logic. I'd appreciate your insights.

Current Behavior Analysis

The skeleton code operates as follows:

retry:
    if (dev->ongoing_read) {
        // Check O_NONBLOCK and wait
        if (file->f_flags & O_NONBLOCK)
            return -EAGAIN;
        wait_event_interruptible(dev->bulk_in_wait, (!dev->ongoing_read));
    }
    
    // Copy data
    chunk = min(available, count);
    copy_to_user(buffer, dev->bulk_in_buffer, chunk);
    
    // When data is insufficient
    if (available < count) {
        usb_do_read_io(dev, count - chunk);
        // No goto retry here!
    }
    return chunk;

The Inconsistency

On entry: Waits for ongoing_read (or returns -EAGAIN if O_NONBLOCK)
When data < count: Submits URB but returns immediately without waiting

These two behaviors seem inconsistent.

Proposed Modification

if (available < count) {
    usb_do_read_io(dev, count - chunk);
    goto retry;  // <- Add this
}

This would:

• Handle O_NONBLOCK consistently at the retry label
• Attempt to fulfill the requested count in blocking I/O mode
• Allow signal interruption via wait_event_interruptible

Questions

1. Is the current behavior intentional, or is it simplified for educational purposes?
2. Am I missing any negative side effects of adding goto retry?
3. Is there a USB Bulk transfer characteristic that requires enforcing Short Reads?

I appreciate any advice. Thank you!

[zerohexer]

Hi, The behavior is intentional, not inconsistent.

Looking at the actual source code, the driver handles three cases differently:

1. No data at all → goto retry (must wait, can't return 0 which means EOF)
2. Buffer exhausted → goto retry (same reason)
3. Some data available → return it immediately, prefetch the rest

Why no goto retry in case 3?

• Returning 0 from read() signals EOF to userspace
• When there's no data, the driver must wait—otherwise it would falsely indicate EOF
• When there is data, returning a short read is standard POSIX behavior
• The prefetch (skel_do_read_io) is an optimization for the next read call

Standard Unix read() semantics:

• read() may return fewer bytes than requested—this is normal
• Userspace is expected to loop if it needs exactly N bytes
• This applies to sockets, pipes, and character devices alike

Adding goto retry would work but would change the driver from "return data as soon as available" to "block until buffer is full"—which increases latency unnecessarily.

Because it would need to wait for full buffer.

//Userspace is expected to handle short reads:

  // Standard pattern - userspace loops, not the driver
  ssize_t read_full(int fd, void *buf, size_t count)
  {
      size_t total = 0;
      while (total < count) {
          ssize_t ret = read(fd, buf + total, count - total);
          if (ret < 0)  return ret;   // error
          if (ret == 0) break;        // EOF
          total += ret;
      }
      return total;
  }

The problem:

 User calls: read(fd, buf, 100)
 Buffer has: 30 bytes

  First iteration:
    - Copy 30 bytes to buf[0..29]
    - rv = 30
    - goto retry...

  Second iteration (after new data arrives):
    - Copy to 'buffer' again (buf[0..??])  ← OVERWRITES first 30 bytes!
    - rv = new_chunk                        ← loses the original 30

  The userspace buffer pointer is never advanced. So goto retry would overwrite data already copied.
 
  copy_to_user(buffer, ...);  // First copy: buf[0]
  goto retry;
  copy_to_user(buffer, ...);  // Second copy: buf[0] again! Data corrupted.

Even if you fixed the buffer pointer issue (by advancing it on each iteration), the modified behavior would still be undesirable because it changes the driver's semantics from "return data as available" to "block until full"