Re: [PATCH 4/4] sg: use standard lists for sg_requests

[Christoph Hellwig <hch@xxxxxx>]

>  typedef struct sg_request {	/* SG_MAX_QUEUE requests outstanding per file */
> -	struct sg_request *nextrp;	/* NULL -> tail request (slist) */
> +	struct list_head nextrp;	/* list entry */

s/nextrp/entry/

> @@ -2078,16 +2076,13 @@ static long sg_compat_ioctl(struct file *filp, unsigned int cmd_in, unsigned lon
>  			if (k < SG_MAX_QUEUE) {
>  				memset(rp, 0, sizeof (Sg_request));
>  				rp->parentfp = sfp;
> +				list_add(&rp->nextrp, &sfp->rq_list);

The old code did a tail insertation.  And this whole function should
become a lot simpler with proper lists anyway:

static Sg_request *
sg_add_request(Sg_fd * sfp)
{
	int k;
	unsigned long iflags;
	Sg_request *rp = sfp->req_arr;

	write_lock_irqsave(&sfp->rq_list_lock, iflags);
	if (!list_empty(&sfp->rq_list)) {
		if (!sfp->cmd_q)
			goto out_unlock;

		for (k = 0; k < SG_MAX_QUEUE; ++k, ++rp) {
			if (!rp->parentfp)
				break;
		}
		if (k >= SG_MAX_QUEUE)
			goto out_unlock;
	}

	memset(rp, 0, sizeof (Sg_request));
	rp->parentfp = sfp;
	rp->header.duration = jiffies_to_msecs(jiffies);
	list_add_tail(&rp->nextrp, &sfp->rq_list);
	write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
	return rp;

out_unlock:
	write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
	return NULL;

> +	if ((!sfp) || (!srp) || (list_empty(&sfp->rq_list)))

No need for all these braces.

> +	if (!list_empty(&srp->nextrp)) {
> +		list_del_init(&srp->nextrp);

I don't think we need the _init as we never check for an empty entry.

>  {
>  	struct sg_fd *sfp = container_of(work, struct sg_fd, ew.work);
>  	struct sg_device *sdp = sfp->parentdp;
> +	Sg_request *srp, *tmp;
>  
>  	/* Cleanup any responses which were never read(). */
> -	while (sfp->headrp)
> -		sg_finish_rem_req(sfp->headrp);
> +	list_for_each_entry_safe(srp, tmp, &sfp->rq_list, nextrp)
> +		sg_finish_rem_req(srp);

What protects us from concurrent removals here?

Either way I'd rather keep the whіle not emptry style even with
proper lists.