[linux-dvb] [PATCH] 2/3: implement DMX_SET_BUFFER_SIZE for dvr

[Oliver Endriss]

Andrea wrote:
> linux-dvb-request at linuxtv.org wrote:
huh?

>  > What about this fragment:
>  > 	...
>  > 	if (!size)
>  > 		return -EINVAL;
>  >
>  > 	mem = vmalloc(size);
>  > 	if (!mem)
>  > 		return -ENOMEM;
>  >
>  > 	mem2 = buf->data;
>  >
>  >       spin_lock_irqsave(&dmxdev->lock);
>  >       buf->pread = buf->pwrite = 0;
>  > 	buf->data = mem;
>  > 	buf->size = size;
>  > 	spin_unlock_irqrestore(&dmxdev->lock);
>  >
>  > 	vfree(mem2);
>  > 	return 0;
> 
> Maybe I can think of one reason while the current code is not implemented this way:
> 
> In your version the new buffer is allocated before the old one is released.

Yes, this is intentional. See below.

> In the current implementation the old buffer is released and afterwards the new one allocated.
> 
> One could argue that the new implementation has a maximum memory requirement higher than the old one.
> It's not much but I am not too familiar with kernel development, so I don't know how important that 
> could be.
> 
> What do you think?

- With your code the demux becomes unusable if the memory allocation
  failes for some reason. This should be avoided. It is better have a
  working demux with a smaller buffer than to have an defunct demux.

- If there is not enough memory for both buffers, your machine has a problem
  anyway, and you should not increase buffer size.

> About the spin_lock_irqsave: currently it is not used anywhere in the code for the demux in dmxdev.c.
> I am always a bit scared when I introduce something new, maybe I am missing the current logic.

I'm sorry, spin_lock_irqsave/spin_unlock_irqrestore was a typo.
We have to use spin_[un]lock_irq because buffer writing _might_ occur
in interrupt context. So the '_irq' is very important!

CU
Oliver

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------