2013-10-11 10:02:10

by Youquan Song

[permalink] [raw]
Subject: DMA: Calculate how many data transferred by DMA

Currently, the DMA channel calculates its data transferred only at network
device driver. When other devices like UART or SPI etc, transfers data by DMA
mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
It will possibly mislead user that the DMA engine does not work.

This patch add a new function which will calculate how many the data has been
transferred after doing it by DMA mode. It can be used by other modules and
also simplify current duplicated code.

Add the interface when UART transfer data by Designware DMA engine. It will
calculate the data already tranferred in the DMA channel.

If the patch work, I will add the interface to other modules when needed.


2013-10-11 10:02:17

by Youquan Song

[permalink] [raw]
Subject: [PATCH 1/2] dma: Add interface to calculate data transferred

Currently, the DMA channel calculates its data transferred only at network
device driver. When other devices like UART or SPI etc, transfers data by DMA
mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.

This patch add a new function which will calculate how many the data has been
transferred after doing it by DMA mode. It can be used by other modules and
also simplify current duplicated code.

Signed-off-by: Youquan Song <[email protected]>
---
drivers/dma/dmaengine.c | 35 +++++++++++++++++++----------------
include/linux/dmaengine.h | 3 +++
2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9162ac8..4356a7e 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -901,6 +901,23 @@ void dma_async_device_unregister(struct dma_device *device)
}
EXPORT_SYMBOL(dma_async_device_unregister);

+dma_cookie_t
+dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
+ struct dma_chan *chan, size_t len)
+{
+
+ dma_cookie_t cookie;
+ cookie = tx->tx_submit(tx);
+
+ preempt_disable();
+ __this_cpu_add(chan->local->bytes_transferred, len);
+ __this_cpu_inc(chan->local->memcpy_count);
+ preempt_enable();
+
+ return cookie;
+
+}
+
/**
* dma_async_memcpy_buf_to_buf - offloaded copy between virtual addresses
* @chan: DMA channel to offload copy to
@@ -920,7 +937,6 @@ dma_async_memcpy_buf_to_buf(struct dma_chan *chan, void *dest,
struct dma_device *dev = chan->device;
struct dma_async_tx_descriptor *tx;
dma_addr_t dma_dest, dma_src;
- dma_cookie_t cookie;
unsigned long flags;

dma_src = dma_map_single(dev->dev, src, len, DMA_TO_DEVICE);
@@ -937,14 +953,8 @@ dma_async_memcpy_buf_to_buf(struct dma_chan *chan, void *dest,
}

tx->callback = NULL;
- cookie = tx->tx_submit(tx);
-
- preempt_disable();
- __this_cpu_add(chan->local->bytes_transferred, len);
- __this_cpu_inc(chan->local->memcpy_count);
- preempt_enable();

- return cookie;
+ return dma_tx_submit_cal(tx, chan, len);
}
EXPORT_SYMBOL(dma_async_memcpy_buf_to_buf);

@@ -968,7 +978,6 @@ dma_async_memcpy_buf_to_pg(struct dma_chan *chan, struct page *page,
struct dma_device *dev = chan->device;
struct dma_async_tx_descriptor *tx;
dma_addr_t dma_dest, dma_src;
- dma_cookie_t cookie;
unsigned long flags;

dma_src = dma_map_single(dev->dev, kdata, len, DMA_TO_DEVICE);
@@ -983,14 +992,8 @@ dma_async_memcpy_buf_to_pg(struct dma_chan *chan, struct page *page,
}

tx->callback = NULL;
- cookie = tx->tx_submit(tx);

- preempt_disable();
- __this_cpu_add(chan->local->bytes_transferred, len);
- __this_cpu_inc(chan->local->memcpy_count);
- preempt_enable();
-
- return cookie;
+ return dma_tx_submit_cal(tx, chan, len);
}
EXPORT_SYMBOL(dma_async_memcpy_buf_to_pg);

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 0bc7275..0025f8e 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1084,4 +1084,7 @@ dma_cookie_t dma_memcpy_pg_to_iovec(struct dma_chan *chan, struct iovec *iov,
struct dma_pinned_list *pinned_list, struct page *page,
unsigned int offset, size_t len);

+dma_cookie_t dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
+ struct dma_chan *chan, size_t len);
+
#endif /* DMAENGINE_H */
--
1.7.7.4

2013-10-11 10:02:16

by Youquan Song

[permalink] [raw]
Subject: [PATCH 2/2] dma: calculate the data tranferred by 8250

When using UART transfers data by DMA mode, but it always shows 0 at
/sys/class/dma/dma0chan*/bytes_transferred.

Call the new function to calculate how many the data has been transferred
after doing it by DMA mode.

Signed-off-by: Youquan Song <[email protected]>
---
drivers/tty/serial/8250/8250_dma.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 7046769..b22ef80 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -83,7 +83,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
desc->callback = __dma_tx_complete;
desc->callback_param = p;

- dma->tx_cookie = dmaengine_submit(desc);
+ dma->tx_cookie = dma_tx_submit_cal(desc, dma->txchan, dma->tx_size);

dma_sync_single_for_device(dma->txchan->device->dev, dma->tx_addr,
UART_XMIT_SIZE, DMA_TO_DEVICE);
--
1.7.7.4

2013-10-11 10:22:47

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Fri, 2013-10-11 at 17:42 -0400, Youquan Song wrote:
> Currently, the DMA channel calculates its data transferred only at network
> device driver. When other devices like UART or SPI etc, transfers data by DMA
> mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
>
> This patch add a new function which will calculate how many the data has been
> transferred after doing it by DMA mode. It can be used by other modules and
> also simplify current duplicated code.

Thanks for the patch. My comments below.

First of all, what is the point to have every device driver that uses
DMA to increment bytes_transferred value? It will show just amount of
data transferred through certain channel. How it could be used then?

> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -901,6 +901,23 @@ void dma_async_device_unregister(struct dma_device *device)
> }
> EXPORT_SYMBOL(dma_async_device_unregister);
>
> +dma_cookie_t
> +dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
> + struct dma_chan *chan, size_t len)

I think there is a better name for the function.
dmaengine_submit_and_count() for example?

> +{
> +
> + dma_cookie_t cookie;

Above lines probably have to be exchanged.

> + cookie = tx->tx_submit(tx);

And you may incorporate this line into above.

> +
> + preempt_disable();
> + __this_cpu_add(chan->local->bytes_transferred, len);
> + __this_cpu_inc(chan->local->memcpy_count);
> + preempt_enable();
> +
> + return cookie;
> +

Redundant empty line.

> +}

--
Andy Shevchenko <[email protected]>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-10-11 13:33:57

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] dma: calculate the data tranferred by 8250

On Fri, Oct 11, 2013 at 05:42:18PM -0400, Youquan Song wrote:
> When using UART transfers data by DMA mode, but it always shows 0 at
> /sys/class/dma/dma0chan*/bytes_transferred.
>
> Call the new function to calculate how many the data has been transferred
> after doing it by DMA mode.

How nice, you just leaked to userspace the size of passwords and other
sensitive things typed into a serial console :(

No, we can't do this, we fixed up this problem in other places we leaked
the transmition of serial data through proc and sysfs files, let's not
add new ones and go backwards in security.

Sorry, we can't do this, and I really doubt that it matters.

greg k-h

2013-10-11 13:34:18

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> Currently, the DMA channel calculates its data transferred only at network
> device driver. When other devices like UART or SPI etc, transfers data by DMA
> mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.

Is that really a problem? I have never heard anyone complaining about
it. Where are the reports of this?

> This patch add a new function which will calculate how many the data has been
> transferred after doing it by DMA mode. It can be used by other modules and
> also simplify current duplicated code.
>
> Signed-off-by: Youquan Song <[email protected]>
> ---
> drivers/dma/dmaengine.c | 35 +++++++++++++++++++----------------
> include/linux/dmaengine.h | 3 +++
> 2 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 9162ac8..4356a7e 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -901,6 +901,23 @@ void dma_async_device_unregister(struct dma_device *device)
> }
> EXPORT_SYMBOL(dma_async_device_unregister);
>
> +dma_cookie_t
> +dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
> + struct dma_chan *chan, size_t len)
> +{
> +
> + dma_cookie_t cookie;
> + cookie = tx->tx_submit(tx);
> +
> + preempt_disable();
> + __this_cpu_add(chan->local->bytes_transferred, len);
> + __this_cpu_inc(chan->local->memcpy_count);
> + preempt_enable();
> +
> + return cookie;
> +
> +}

You create a function, yet no driver can use it as it's not exported, so
I guess you don't want anyone to use it :)

Anyway, I don't think this is a good idea, see my response to the serial
driver patch for why not.

sorry,

greg k-h

2013-10-13 16:19:22

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > Currently, the DMA channel calculates its data transferred only at network
> > device driver. When other devices like UART or SPI etc, transfers data by DMA
> > mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
>
> Is that really a problem? I have never heard anyone complaining about
> it. Where are the reports of this?
Right, am not still getting the point on what is the problem that this series is
trying to fix..

~Vinod

2013-10-15 06:31:25

by Youquan Song

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > Currently, the DMA channel calculates its data transferred only at network
> > > device driver. When other devices like UART or SPI etc, transfers data by DMA
> > > mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
> >
> > Is that really a problem? I have never heard anyone complaining about
> > it. Where are the reports of this?
> Right, am not still getting the point on what is the problem that this series is
> trying to fix..

The issue is that when I using UART to transfer data between to COMs
which using Designware DMA controller channel. But I check the specific
DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
should all "0". I have transferred data by UART port, why its DMA
channel report "0" bytes transferred? So I guess that it is possible
the DMA device driver issue or the data does not use the Designware DMA channel
fro transferred. After check the code, I notice only when the DMA
channel used by network device driver and it will record how much data has been
tranferred, why other device driver will not calculate it. Since DMA
channel is used by other device driver, why only network is specific? since it is
common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
much possibility to mislead the user.


Thanks
-Youquan

2013-10-15 15:30:06

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Tue, Oct 15, 2013 at 02:31:42PM -0400, Youquan Song wrote:
> On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> > On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > > Currently, the DMA channel calculates its data transferred only at network
> > > > device driver. When other devices like UART or SPI etc, transfers data by DMA
> > > > mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
> > >
> > > Is that really a problem? I have never heard anyone complaining about
> > > it. Where are the reports of this?
> > Right, am not still getting the point on what is the problem that this series is
> > trying to fix..
>
> The issue is that when I using UART to transfer data between to COMs
> which using Designware DMA controller channel. But I check the specific
> DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> should all "0". I have transferred data by UART port, why its DMA
> channel report "0" bytes transferred? So I guess that it is possible
> the DMA device driver issue or the data does not use the Designware DMA channel
> fro transferred. After check the code, I notice only when the DMA
> channel used by network device driver and it will record how much data has been
> tranferred, why other device driver will not calculate it. Since DMA
> channel is used by other device driver, why only network is specific? since it is
> common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> much possibility to mislead the user.

If you are worried about the data transferred through a UART, then look
at the accounting variables for that device (in /proc/ ) and use them,
they should be a good enough hint to see that data is flowing, not the
dma data.

And again, I can't take this due to the security implications, we
already went down this path with the TTY bytes, why do you want us to
introduce the same issue again only to have to fix it again?

greg k-h

2013-10-15 15:55:26

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
<[email protected]> wrote:
> On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
>> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
>> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> The issue is that when I using UART to transfer data between to COMs
> which using Designware DMA controller channel. But I check the specific
> DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> should all "0". I have transferred data by UART port, why its DMA
> channel report "0" bytes transferred? So I guess that it is possible
> the DMA device driver issue or the data does not use the Designware DMA channel
> fro transferred. After check the code, I notice only when the DMA
> channel used by network device driver and it will record how much data has been
> tranferred, why other device driver will not calculate it. Since DMA
> channel is used by other device driver, why only network is specific? since it is
> common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> much possibility to mislead the user.

Yes, and for that reason I think we should delete "
/sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
purpose besides "is my dma channel working" which can be determined by
other means.

--
Dan

2013-10-15 16:17:09

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> <[email protected]> wrote:
> > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > The issue is that when I using UART to transfer data between to COMs
> > which using Designware DMA controller channel. But I check the specific
> > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > should all "0". I have transferred data by UART port, why its DMA
> > channel report "0" bytes transferred? So I guess that it is possible
> > the DMA device driver issue or the data does not use the Designware DMA channel
> > fro transferred. After check the code, I notice only when the DMA
> > channel used by network device driver and it will record how much data has been
> > tranferred, why other device driver will not calculate it. Since DMA
> > channel is used by other device driver, why only network is specific? since it is
> > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > much possibility to mislead the user.
>
> Yes, and for that reason I think we should delete "
> /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> purpose besides "is my dma channel working" which can be determined by
> other means.

Sounds good to me, feel free to send a patch.

thanks,

greg k-h

2013-10-16 06:31:59

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> <[email protected]> wrote:
> > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > The issue is that when I using UART to transfer data between to COMs
> > which using Designware DMA controller channel. But I check the specific
> > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > should all "0". I have transferred data by UART port, why its DMA
> > channel report "0" bytes transferred? So I guess that it is possible
> > the DMA device driver issue or the data does not use the Designware DMA channel
> > fro transferred. After check the code, I notice only when the DMA
> > channel used by network device driver and it will record how much data has been
> > tranferred, why other device driver will not calculate it. Since DMA
> > channel is used by other device driver, why only network is specific? since it is
> > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > much possibility to mislead the user.
>
> Yes, and for that reason I think we should delete "
> /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> purpose besides "is my dma channel working" which can be determined by
> other means.
Well am going to take it a bit further and ask you why do we need the
/sys/class/dma? I have never used it for slave work.

Does it get used anywhere in async_tx?

--
~Vinod

2013-10-16 08:36:40

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
> On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> > On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> > <[email protected]> wrote:
> > > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> > >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > The issue is that when I using UART to transfer data between to COMs
> > > which using Designware DMA controller channel. But I check the specific
> > > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > > should all "0". I have transferred data by UART port, why its DMA
> > > channel report "0" bytes transferred? So I guess that it is possible
> > > the DMA device driver issue or the data does not use the Designware DMA channel
> > > fro transferred. After check the code, I notice only when the DMA
> > > channel used by network device driver and it will record how much data has been
> > > tranferred, why other device driver will not calculate it. Since DMA
> > > channel is used by other device driver, why only network is specific? since it is
> > > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > > much possibility to mislead the user.
> >
> > Yes, and for that reason I think we should delete "
> > /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> > purpose besides "is my dma channel working" which can be determined by
> > other means.
> Well am going to take it a bit further and ask you why do we need the
> /sys/class/dma? I have never used it for slave work.

How user (who, f.e., would like to run dmatest) will know names of the
channels provided?

How could we see what channels of certain dma controller are requested /
busy from userspace?

--
Andy Shevchenko <[email protected]>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-10-16 08:50:10

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
> > On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> > > On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> > > <[email protected]> wrote:
> > > > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> > > >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > > >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > > The issue is that when I using UART to transfer data between to COMs
> > > > which using Designware DMA controller channel. But I check the specific
> > > > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > > > should all "0". I have transferred data by UART port, why its DMA
> > > > channel report "0" bytes transferred? So I guess that it is possible
> > > > the DMA device driver issue or the data does not use the Designware DMA channel
> > > > fro transferred. After check the code, I notice only when the DMA
> > > > channel used by network device driver and it will record how much data has been
> > > > tranferred, why other device driver will not calculate it. Since DMA
> > > > channel is used by other device driver, why only network is specific? since it is
> > > > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > > > much possibility to mislead the user.
> > >
> > > Yes, and for that reason I think we should delete "
> > > /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> > > purpose besides "is my dma channel working" which can be determined by
> > > other means.
> > Well am going to take it a bit further and ask you why do we need the
> > /sys/class/dma? I have never used it for slave work.
>
> How user (who, f.e., would like to run dmatest) will know names of the
> channels provided?
Ok dmatest requires this, I overlooked that part

> How could we see what channels of certain dma controller are requested /
> busy from userspace?
But do end user care or need to know about this?

--
~Vinod

2013-10-16 09:13:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
> On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:

[]

> > > Well am going to take it a bit further and ask you why do we need the
> > > /sys/class/dma? I have never used it for slave work.
> >
> > How user (who, f.e., would like to run dmatest) will know names of the
> > channels provided?
> Ok dmatest requires this, I overlooked that part
>
> > How could we see what channels of certain dma controller are requested /
> > busy from userspace?
> But do end user care or need to know about this?

The idea is to have some test cases. So, end user probably doesn't
really need this.

--
Andy Shevchenko <[email protected]>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-10-16 14:11:48

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Wed, Oct 16, 2013 at 09:13:20AM +0000, Shevchenko, Andriy wrote:
> On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
> > On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> > > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
>
> []
>
> > > > Well am going to take it a bit further and ask you why do we need the
> > > > /sys/class/dma? I have never used it for slave work.
> > >
> > > How user (who, f.e., would like to run dmatest) will know names of the
> > > channels provided?
> > Ok dmatest requires this, I overlooked that part
> >
> > > How could we see what channels of certain dma controller are requested /
> > > busy from userspace?
> > But do end user care or need to know about this?
>
> The idea is to have some test cases. So, end user probably doesn't
> really need this.

So could this just be debugfs files instead?

2013-10-16 16:00:44

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Wed, Oct 16, 2013 at 07:12:47AM -0700, Greg KH wrote:
> On Wed, Oct 16, 2013 at 09:13:20AM +0000, Shevchenko, Andriy wrote:
> > On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
> > > On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> > > > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
> >
> > []
> >
> > > > > Well am going to take it a bit further and ask you why do we need the
> > > > > /sys/class/dma? I have never used it for slave work.
> > > >
> > > > How user (who, f.e., would like to run dmatest) will know names of the
> > > > channels provided?
> > > Ok dmatest requires this, I overlooked that part
> > >
> > > > How could we see what channels of certain dma controller are requested /
> > > > busy from userspace?
> > > But do end user care or need to know about this?
> >
> > The idea is to have some test cases. So, end user probably doesn't
> > really need this.
>
> So could this just be debugfs files instead?
Yes I was thinking on same lines and would agree with you on that. This _really_
doesn't need sysfs unless Dan some async usage dependent on it. While at it we
can review the fields and remove the ones which are not required...

Dan, let me know if you are okay with it, I will try to get that done over the
weekend...

--
~Vinod

2013-10-16 18:17:37

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 1/2] dma: Add interface to calculate data transferred

On Wed, Oct 16, 2013 at 8:07 AM, Vinod Koul <[email protected]> wrote:
> On Wed, Oct 16, 2013 at 07:12:47AM -0700, Greg KH wrote:
>> On Wed, Oct 16, 2013 at 09:13:20AM +0000, Shevchenko, Andriy wrote:
>> > On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
>> > > On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
>> > > > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
>> >
>> > []
>> >
>> > > > > Well am going to take it a bit further and ask you why do we need the
>> > > > > /sys/class/dma? I have never used it for slave work.
>> > > >
>> > > > How user (who, f.e., would like to run dmatest) will know names of the
>> > > > channels provided?
>> > > Ok dmatest requires this, I overlooked that part
>> > >
>> > > > How could we see what channels of certain dma controller are requested /
>> > > > busy from userspace?
>> > > But do end user care or need to know about this?
>> >
>> > The idea is to have some test cases. So, end user probably doesn't
>> > really need this.
>>
>> So could this just be debugfs files instead?
> Yes I was thinking on same lines and would agree with you on that. This _really_
> doesn't need sysfs unless Dan some async usage dependent on it. While at it we
> can review the fields and remove the ones which are not required...
>
> Dan, let me know if you are okay with it, I will try to get that done over the
> weekend...
>

Channel naming is the primary use case and several drivers rely on the
channel name in their print messages, some for naming resources, and
of course dmatest for identifying channels. ioatdma uses it as a
parent device for allocating its per-channel 'quickdata' attributes to
advertise the capabilities and the ring_size which is useful for
determining channel loading and verifying the BIOS configuration.

An argument could be made to move the ioatdma attributes to debugfs,
but I don't see a clean/worthwhile way of ripping and replacing the
channel naming scheme for drivers.

--
Dan