Discussion:
[etherlab-dev] Pre-announcement: unofficial patchset update (Gavin Lambert)
Knud Baastrup
2017-04-27 11:10:55 UTC
Permalink
Hi Gavin,

I am also in favor of #3 and suggest that naming is "auto generated" based on the first line of the commit message like done with git-format-patch.

"By default, each output file is numbered sequentially from 1, and uses the first line of the commit message (massaged for pathname safety) as the filename." (https://git-scm.com/docs/git-format-patch)


It could be great if it was possible to maintain backwards compatibility in the patch-serie or alternatively ensure that breaking patches are put on top or in a separate serie.


BR, Knud


-----Original Message-----
From: etherlab-dev [mailto:etherlab-dev-***@etherlab.org] On Behalf Of etherlab-dev-***@etherlab.org
Sent: 27. april 2017 12:00
To: etherlab-***@etherlab.org
Subject: etherlab-dev Digest, Vol 93, Issue 3

Send etherlab-dev mailing list submissions to
etherlab-***@etherlab.org

To subscribe or unsubscribe via the World Wide Web, visit
http://lists.etherlab.org/mailman/listinfo/etherlab-dev
or, via email, send a message with subject or body 'help' to
etherlab-dev-***@etherlab.org

You can reach the person managing the list at
etherlab-dev-***@etherlab.org

When replying, please edit your Subject line so it is more specific than "Re: Contents of etherlab-dev digest..."


Today's Topics:

1. Pre-announcement: unofficial patchset update (Gavin Lambert)


----------------------------------------------------------------------

Message: 1
Date: Thu, 27 Apr 2017 19:31:21 +1200
From: Gavin Lambert <***@compacsort.com>
To: <etherlab-***@etherlab.org>
Subject: [etherlab-dev] Pre-announcement: unofficial patchset update
Message-ID: <000001d2bf28$47333120$d5999360$@compacsort.com>
Content-Type: text/plain; charset="us-ascii"

Hi all,

I've recently been doing some more work on my local Etherlab-related code, and as a result I'm planning to "shortly" (by which I mean still probably a few weeks away) release a new version of my default-branch unofficial patchset.

I thought it would be a good idea to let everyone know in advance both to gather any new patches people might want to add (or feedback on or suggested changes to existing patches), but also since this is the first update since publishing it as a repository I'd like to know people's preferences with regard to re-ordering patches from the existing set. For example, I could:

1. Retain existing patches exactly as is (barring fuzz updates) and only add new patches (modifications to existing patches are added as a new patch).
2. Retain existing patches with the same numbering but allow both new patches (with strictly higher numbers) and modifying existing patches.
3. Renumber existing patches as needed to insert new patches in a logical place (either grouping patches by related function or putting the simplest patches first so they have fewer dependencies and are thus hopefully easier to get included into upstream).
4. Abandon the idea of numbered patches entirely and just rely on consistent names plus the series file to maintain order. (Thus also allowing new patches to be inserted wherever seems sensible.)

If I don't get any feedback, I'm probably inclined towards #3 at this point, but I might go to #4 if it looks tricky to maintain patch history properly with #3. I could be persuaded towards #2 (though it's not my preference) but am disinclined towards #1. Or maybe there's some alternate method I haven't considered.

I might see if I can group related patches into subdirectories -- I know quilt/Debian patchsets support this, though I'm not sure about HG patchsets.

(FYI, I've already made a note of the patch updates Knud Baastrup submitted in December, and a few other patches posted to the list since then, though I haven't integrated them yet.)

Regards,
Gavin Lambert




------------------------------

Subject: Digest Footer

_______________________________________________
etherlab-dev mailing list
etherlab-***@etherlab.org
http://lists.etherlab.org/mailman/listinfo/etherlab-dev


------------------------------

End of etherlab-dev Digest, Vol 93, Issue 3
*******************************************
Graeme Foot
2017-04-27 23:59:06 UTC
Permalink
Hi,

I just had a play with your patchset the other day to try to resolve some very slow SDO read/write timings.

First off, I also prefer #3. I use buildroot to create my Linux environment and buildroot applies patches in alphabetical order (at least in my version which is now pretty old), so the number at the front is important. Buildroot also requires that the patch starts with the name of the package (and optionally a revision number), but that is easy for me to prefix.


I would prefer breaking change patches to be last in the list if possible.


When I tried to use your patchset (and the EtherCAT revision they were for) the computer would freeze just after starting my application and going realtime. We use RTAI and I read your notes re that it wasn't tested with RTAI. I didn't have much time to look into problem but I suspect there may have been a lock that ended up in a call from the realtime context that was blocked due to be held in the master thread. I ended up cherry picking the changes I needed (patch 0054 (sdo requests via slave fsm) and 0038 (sdo write request)).

FYI: We are still using Linux kernel 2.6.32.11 and had a compilation issue with one of the patches due to header file differences. Unfortunately I seem to have deleted the info as to what it was.


As to other potential patches (Note: my patches are against 2526 (2eff7c993a63) on the stable-1.5 branch):

1) RTAI has issues with fprintf calls from the realtime context. It should instead use rt_printk. My patch "etherlabmaster-0001-replace_fprintf_calls_with_EC_PRINT_ERR.patch" defines a macro that replaces fprintf calls if USE_RTDM is defined.


2) Your 0054 patch successfully moved the slave sdo request processing from the master fsm's idle state to be called every cycle of the master thread. However, I have my Linux environment set to run at 100Hz, so this thread would only fire once every 10ms. Each SDO read/write request would take approx. 30ms to complete. I didn't want to change Linux to run at 1000Hz so I created patch "etherlabmaster-0010-allow_app_to_process_sdo_requests_from_realtime.patch".

This patch optionally allows the slave request processing to be called from within my application in the realtime thread. I added two new functions, one to set whether slave request processing is expected to be processed by the master thread as usual, or by your app. The second function needs to be called periodically by your app. Note: if the master is idle (ie your app is not running) then sdo request processing will be handled as normal. In my app I process the slave requests once every 2ms (every second realtime cycle) at the end of the realtime cycle. At this rate, SDO read/writes are handled within 6ms (a couple of extra ms due to other application timings).


3) One last patch you may be interested in. I was having problems with my distributed clock setup. Some changes I had made a while ago caused my system to have a very/very slow drift (approx 1ms over 63 days). Every now and then our machines would start running roughly which a repower would resolve. It turned out to be due to the machines not having been repowered for a few weeks and the drift would eventually cause the EtherCAT frame to be sent out at about the time of the DC sync events in the slaves and due to jitter the frames randomly falling either side of the event. To help diagnose this I added a function to queue and read the dc reference slave clocks 64bit time. Effectively it does nothing except put the whole 64bit clock time into the EtherCAT frame so that wireshark has a long running time reference (rather than just the 32bit low part that rolls over every 4 odd seconds). Once I put that in, the drift (even though very small) became very obvious. (etherlabmaster-0008-read_reference_slave_clock_64bit_time.patch)


It takes a fair bit of time creating and maintaining patches so everyone's effort has been much appreciated.


Regards,
Graeme Foot


-----Original Message-----
From: etherlab-dev [mailto:etherlab-dev-***@etherlab.org] On Behalf Of Knud Baastrup
Sent: Thursday, 27 April 2017 11:11 p.m.
To: Gavin Lambert <***@compacsort.com>
Cc: etherlab-***@etherlab.org
Subject: Re: [etherlab-dev] Pre-announcement: unofficial patchset update (Gavin Lambert)

Hi Gavin,

I am also in favor of #3 and suggest that naming is "auto generated" based on the first line of the commit message like done with git-format-patch.

"By default, each output file is numbered sequentially from 1, and uses the first line of the commit message (massaged for pathname safety) as the filename." (https://git-scm.com/docs/git-format-patch)


It could be great if it was possible to maintain backwards compatibility in the patch-serie or alternatively ensure that breaking patches are put on top or in a separate serie.


BR, Knud


-----Original Message-----
From: etherlab-dev [mailto:etherlab-dev-***@etherlab.org] On Behalf Of etherlab-dev-***@etherlab.org
Sent: 27. april 2017 12:00
To: etherlab-***@etherlab.org
Subject: etherlab-dev Digest, Vol 93, Issue 3

Send etherlab-dev mailing list submissions to
etherlab-***@etherlab.org

To subscribe or unsubscribe via the World Wide Web, visit
http://lists.etherlab.org/mailman/listinfo/etherlab-dev
or, via email, send a message with subject or body 'help' to
etherlab-dev-***@etherlab.org

You can reach the person managing the list at
etherlab-dev-***@etherlab.org

When replying, please edit your Subject line so it is more specific than "Re: Contents of etherlab-dev digest..."


Today's Topics:

1. Pre-announcement: unofficial patchset update (Gavin Lambert)


----------------------------------------------------------------------

Message: 1
Date: Thu, 27 Apr 2017 19:31:21 +1200
From: Gavin Lambert <***@compacsort.com>
To: <etherlab-***@etherlab.org>
Subject: [etherlab-dev] Pre-announcement: unofficial patchset update
Message-ID: <000001d2bf28$47333120$d5999360$@compacsort.com>
Content-Type: text/plain; charset="us-ascii"

Hi all,

I've recently been doing some more work on my local Etherlab-related code, and as a result I'm planning to "shortly" (by which I mean still probably a few weeks away) release a new version of my default-branch unofficial patchset.

I thought it would be a good idea to let everyone know in advance both to gather any new patches people might want to add (or feedback on or suggested changes to existing patches), but also since this is the first update since publishing it as a repository I'd like to know people's preferences with regard to re-ordering patches from the existing set. For example, I could:

1. Retain existing patches exactly as is (barring fuzz updates) and only add new patches (modifications to existing patches are added as a new patch).
2. Retain existing patches with the same numbering but allow both new patches (with strictly higher numbers) and modifying existing patches.
3. Renumber existing patches as needed to insert new patches in a logical place (either grouping patches by related function or putting the simplest patches first so they have fewer dependencies and are thus hopefully easier to get included into upstream).
4. Abandon the idea of numbered patches entirely and just rely on consistent names plus the series file to maintain order. (Thus also allowing new patches to be inserted wherever seems sensible.)

If I don't get any feedback, I'm probably inclined towards #3 at this point, but I might go to #4 if it looks tricky to maintain patch history properly with #3. I could be persuaded towards #2 (though it's not my preference) but am disinclined towards #1. Or maybe there's some alternate method I haven't considered.

I might see if I can group related patches into subdirectories -- I know quilt/Debian patchsets support this, though I'm not sure about HG patchsets.

(FYI, I've already made a note of the patch updates Knud Baastrup submitted in December, and a few other patches posted to the list since then, though I haven't integrated them yet.)

Regards,
Gavin Lambert




------------------------------

Subject: Digest Footer

_______________________________________________
etherlab-dev mailing list
etherlab-***@etherlab.org
http://lists.etherlab.org/mailman/listinfo/etherlab-dev


------------------------------

End of etherlab-dev Digest, Vol 93, Issue 3
*******************************************
Ricardo Ribalda Delgado
2017-04-28 07:25:47 UTC
Permalink
Sorry for entering a bit late the conversation.

If I were you I would just create a repository on github and add
patches to your repo as you wish. Whoever wants to see the history can
do a git log of a file and see the history of the file.

When there is a new upstream version rebase the whole patcheset on the
top of the new repo. I can help you with git if you need.

For people that are using build systems they just have to point their
builders to a specific version of your repo.

Also attached you will find a patch for FOE.

Thanks for your effort!
Post by Graeme Foot
Hi,
I just had a play with your patchset the other day to try to resolve some very slow SDO read/write timings.
First off, I also prefer #3. I use buildroot to create my Linux environment and buildroot applies patches in alphabetical order (at least in my version which is now pretty old), so the number at the front is important. Buildroot also requires that the patch starts with the name of the package (and optionally a revision number), but that is easy for me to prefix.
I would prefer breaking change patches to be last in the list if possible.
When I tried to use your patchset (and the EtherCAT revision they were for) the computer would freeze just after starting my application and going realtime. We use RTAI and I read your notes re that it wasn't tested with RTAI. I didn't have much time to look into problem but I suspect there may have been a lock that ended up in a call from the realtime context that was blocked due to be held in the master thread. I ended up cherry picking the changes I needed (patch 0054 (sdo requests via slave fsm) and 0038 (sdo write request)).
FYI: We are still using Linux kernel 2.6.32.11 and had a compilation issue with one of the patches due to header file differences. Unfortunately I seem to have deleted the info as to what it was.
1) RTAI has issues with fprintf calls from the realtime context. It should instead use rt_printk. My patch "etherlabmaster-0001-replace_fprintf_calls_with_EC_PRINT_ERR.patch" defines a macro that replaces fprintf calls if USE_RTDM is defined.
2) Your 0054 patch successfully moved the slave sdo request processing from the master fsm's idle state to be called every cycle of the master thread. However, I have my Linux environment set to run at 100Hz, so this thread would only fire once every 10ms. Each SDO read/write request would take approx. 30ms to complete. I didn't want to change Linux to run at 1000Hz so I created patch "etherlabmaster-0010-allow_app_to_process_sdo_requests_from_realtime.patch".
This patch optionally allows the slave request processing to be called from within my application in the realtime thread. I added two new functions, one to set whether slave request processing is expected to be processed by the master thread as usual, or by your app. The second function needs to be called periodically by your app. Note: if the master is idle (ie your app is not running) then sdo request processing will be handled as normal. In my app I process the slave requests once every 2ms (every second realtime cycle) at the end of the realtime cycle. At this rate, SDO read/writes are handled within 6ms (a couple of extra ms due to other application timings).
3) One last patch you may be interested in. I was having problems with my distributed clock setup. Some changes I had made a while ago caused my system to have a very/very slow drift (approx 1ms over 63 days). Every now and then our machines would start running roughly which a repower would resolve. It turned out to be due to the machines not having been repowered for a few weeks and the drift would eventually cause the EtherCAT frame to be sent out at about the time of the DC sync events in the slaves and due to jitter the frames randomly falling either side of the event. To help diagnose this I added a function to queue and read the dc reference slave clocks 64bit time. Effectively it does nothing except put the whole 64bit clock time into the EtherCAT frame so that wireshark has a long running time reference (rather than just the 32bit low part that rolls over every 4 odd seconds). Once I put that in, the drift (even though very small) became very obvious. (etherlabmaster-0008-read_reference_slave_clock_64bit_time.patch)
It takes a fair bit of time creating and maintaining patches so everyone's effort has been much appreciated.
Regards,
Graeme Foot
-----Original Message-----
Sent: Thursday, 27 April 2017 11:11 p.m.
Subject: Re: [etherlab-dev] Pre-announcement: unofficial patchset update (Gavin Lambert)
Hi Gavin,
I am also in favor of #3 and suggest that naming is "auto generated" based on the first line of the commit message like done with git-format-patch.
"By default, each output file is numbered sequentially from 1, and uses the first line of the commit message (massaged for pathname safety) as the filename." (https://git-scm.com/docs/git-format-patch)
It could be great if it was possible to maintain backwards compatibility in the patch-serie or alternatively ensure that breaking patches are put on top or in a separate serie.
BR, Knud
-----Original Message-----
Sent: 27. april 2017 12:00
Subject: etherlab-dev Digest, Vol 93, Issue 3
Send etherlab-dev mailing list submissions to
To subscribe or unsubscribe via the World Wide Web, visit
http://lists.etherlab.org/mailman/listinfo/etherlab-dev
or, via email, send a message with subject or body 'help' to
You can reach the person managing the list at
When replying, please edit your Subject line so it is more specific than "Re: Contents of etherlab-dev digest..."
1. Pre-announcement: unofficial patchset update (Gavin Lambert)
----------------------------------------------------------------------
Message: 1
Date: Thu, 27 Apr 2017 19:31:21 +1200
Subject: [etherlab-dev] Pre-announcement: unofficial patchset update
Content-Type: text/plain; charset="us-ascii"
Hi all,
I've recently been doing some more work on my local Etherlab-related code, and as a result I'm planning to "shortly" (by which I mean still probably a few weeks away) release a new version of my default-branch unofficial patchset.
1. Retain existing patches exactly as is (barring fuzz updates) and only add new patches (modifications to existing patches are added as a new patch).
2. Retain existing patches with the same numbering but allow both new patches (with strictly higher numbers) and modifying existing patches.
3. Renumber existing patches as needed to insert new patches in a logical place (either grouping patches by related function or putting the simplest patches first so they have fewer dependencies and are thus hopefully easier to get included into upstream).
4. Abandon the idea of numbered patches entirely and just rely on consistent names plus the series file to maintain order. (Thus also allowing new patches to be inserted wherever seems sensible.)
If I don't get any feedback, I'm probably inclined towards #3 at this point, but I might go to #4 if it looks tricky to maintain patch history properly with #3. I could be persuaded towards #2 (though it's not my preference) but am disinclined towards #1. Or maybe there's some alternate method I haven't considered.
I might see if I can group related patches into subdirectories -- I know quilt/Debian patchsets support this, though I'm not sure about HG patchsets.
(FYI, I've already made a note of the patch updates Knud Baastrup submitted in December, and a few other patches posted to the list since then, though I haven't integrated them yet.)
Regards,
Gavin Lambert
------------------------------
Subject: Digest Footer
_______________________________________________
etherlab-dev mailing list
http://lists.etherlab.org/mailman/listinfo/etherlab-dev
------------------------------
End of etherlab-dev Digest, Vol 93, Issue 3
*******************************************
_______________________________________________
etherlab-dev mailing list
http://lists.etherlab.org/mailman/listinfo/etherlab-dev
_______________________________________________
etherlab-dev mailing list
http://lists.etherlab.org/mailman/listinfo/etherlab-dev
--
Ricardo Ribalda
Gavin Lambert
2017-05-05 04:23:38 UTC
Permalink
Post by Graeme Foot
First off, I also prefer #3. I use buildroot to create my Linux
environment and
Post by Graeme Foot
buildroot applies patches in alphabetical order (at least in my version
which is
Post by Graeme Foot
now pretty old), so the number at the front is important. Buildroot also
requires that the patch starts with the name of the package (and
optionally a
Post by Graeme Foot
revision number), but that is easy for me to prefix.
FWIW, it looks like buildroot does follow a series file if one is present,
so you could just take the series file that's supplied with the patchset and
comment or delete from it the patches you're uninterested in, and add
additional ones of your own.
Post by Graeme Foot
When I tried to use your patchset (and the EtherCAT revision they were
for)
Post by Graeme Foot
the computer would freeze just after starting my application and going
realtime. We use RTAI and I read your notes re that it wasn't tested with
RTAI. I didn't have much time to look into problem but I suspect there
may
Post by Graeme Foot
have been a lock that ended up in a call from the realtime context that
was
Post by Graeme Foot
blocked due to be held in the master thread.
I suspect this is probably either patch 0007, patch 0011, or patch 0030. If
you do get a chance to look into it, it'd be good to confirm that. I'm
considering wrapping one or more of these in a configure --enable (or
otherwise disabling them when RTDM is enabled).
Post by Graeme Foot
I ended up cherry picking the changes I needed (patch 0054 (sdo requests
via slave fsm) and 0038 (sdo write request)).
That could be a little dangerous; the 005x patches are highly dependent on
prior patches and it's probably very bad to run 0054 without 0050-0053.
Post by Graeme Foot
As to other potential patches (Note: my patches are against 2526
Thanks, I'll look at including these too.
Post by Graeme Foot
2) Your 0054 patch successfully moved the slave sdo request processing
from
Post by Graeme Foot
the master fsm's idle state to be called every cycle of the master thread.
However, I have my Linux environment set to run at 100Hz, so this thread
would only fire once every 10ms. Each SDO read/write request would take
approx. 30ms to complete. I didn't want to change Linux to run at 1000Hz
so I
Post by Graeme Foot
created patch "etherlabmaster-0010-
allow_app_to_process_sdo_requests_from_realtime.patch".
Maybe I'm missing something about how the RTAI version works, but all I did
was to move the logic from inside fsm_master to inside fsm_slave; AFAIK both
of these execute on the same thread (either the master IDLE or master
OPERATION thread). This is independent of the application realtime loop
either way, so this should not have changed how frequently they run; it just
allows multiple slave requests to run in parallel rather than forcing them
to execute sequentially, so it should be a net performance gain.

Though it does seem reasonable in your use case to want to run the slave
FSMs more often. I don't see how you could do that safely, though -- you
can't run ec_master_exec_slave_fsms outside of the master_sem lock, and your
RTAI task might interrupt Linux while it's still holding that lock, which
will deadlock your RTAI task.

Loading...