Re: [PATCH v4 0/3] USB Audio Gadget refactoring
Hi, On Fri, Jun 2, 2017 at 12:42 PM, Felipe Balbi wrote: > > Hi, > > Ruslan Bilovol writes: >> I came to this patch series when wanted to do two things: >> - use UAC1 as virtual ALSA sound card on gadget side, >>just like UAC2 is used so it's possible to do rate >>resampling >> - have both playback/capture support in UAC1 >> >> Since I wanted to have same behavior for both UAC1/UAC2, >> obviously I've got an utility part (u_audio.c) for >> virtual ALSA sound card handling like we have >> for ethernet(u_ether) or serial(u_serial) functions. >> Function-specific parts (f_uac1/f_uac2) became almost >> as storage for class-specific USB descriptors, some >> boilerplate for configfs, binding and few USB >> config request handling. >> >> Originally in RFC [1] I've posted before, there was >> major change to f_uac1 after that it couldn't do >> direct play to existing ALSA sound card anymore, >> representing audio on gadget side as virtual >> ALSA sound card where audio streams are simply >> sinked to and sourced from it, so it may break >> current usecase for some people (and that's why >> it was RFC). >> >> During RFC discussion, it was agreed to not touch >> existing f_uac1 implementation and create new one >> instead. This patchset (v4) introduced new function >> named f_uac1_acard and doesn't touch current f_uac1 >> implementation, so people still can use old behavior > > Do you have a pointer to the original RFC discussion where this was > discussed? If we really *must* keep the old implementation, I would > rather rename that to f_uac1_legacy. Still, I find it unlikely that > anybody will care about the old implementation. It is on LKML (which is down for me) [1] or alternative archive [2] > >> Now, it's possible to use existing user-space >> applications for audio routing between Audio Gadget >> and real sound card. I personally use alsaloop tool >> from alsautils and have ability to create PCM >> loopback between two different ALSA cards using >> rate resampling, which was not possible with previous >> "direct play to ALSA card" approach in f_uac1. > > this is really good result and will actually make it a lot easier for > testing things out. > >> While here, also dropped redundant platform >> driver/device creation in f_uac2 driver (as well as >> didn't add "never implemented" volume/mute functionality >> in f_uac1 to f_uac1_acard) that made this work even >> easier to do. >> >> This series is tested with both legacy g_audio.ko and >> modern configfs approaches under Ubuntu 14.04 (UAC1 and >> UAC2) and under Windows7 x64 (UAC1 only) having >> perfect results in all cases. >> >> Comments, testing are welcome. >> >> v4 changes: >> - renamed f_uac1_newapi to f_uac1_acard that is >>more meaningful > > I really don't get why you wanna keep both f_uac1 and f_uac1_acard. Why > do we need to maintain the old uac1 implementation? Why two separate > files? In first RFC ([1],[2]) I did exactly what you wrote here (removed old uac1 implementation and replaced it by new one) but got feedback that it will break things for existing f_uac1 legacy users and it's better to have separate implementation. I'm OK with dropping legacy f_uac1 implementation. Another idea I was thinking about is to implement simple in-kernel driver which will do the same as existing alsaloop tool userspace tool does (so legacy users will need to load two kernel modules and get same functionality). But this seems to be a wrong way, since It known that Linux kernel community doesn't like to take drivers with same functionality as existing userspace tools already have. So bottom line: since I'm not a legacy f_uac1 user, there is no difference for me how to handle it - remove legacy f_uac1 completely, rename it to f_uac1_legacy or add separate f_uac1_acard function. So if dropping of legacy f_uac1 implementation is OK for you, I can do it quickly in next patchset. [1] https://lkml.org/lkml/2016/5/23/649 [2] https://marc.info/?t=14640475881&r=1&w=4 Regards, Ruslan -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] usb: gadget: f_uac2: split out audio core
Hi Felipe, On Fri, Jun 2, 2017 at 12:34 PM, Felipe Balbi wrote: > > Hi, > > Ruslan Bilovol writes: >> Abstract the peripheral side ALSA sound card code from >> the f_uac2 function into a component that can be called >> by various functions, so the various flavors can be split >> apart and selectively reused. >> >> Visible changes: >> - add uac_params structure to pass audio paramteres for >>g_audio_setup >> - make ALSA sound card's name configurable >> - add [in/out]_ep_maxpsize >> - allocate snd_uac_chip structure during g_audio_setup >> - add u_audio_[start/stop]_[capture/playback] functions >> >> Signed-off-by: Ruslan Bilovol > > this doesn't apply on testing/next, care to rebase? > sure, I'll rebase it and address comments from Jassi Regards, Ruslan -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] perf: xgene: Add support for SoC PMU version 3
Hi Mark,
On Fri, Jun 2, 2017 at 9:04 AM, Mark Rutland wrote:
> Hi Hoan,
>
> Apologies for the delay in getting to this.
>
> On Mon, Apr 03, 2017 at 09:47:57AM -0700, Hoan Tran wrote:
>> This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
>> Unit version 3.
>>
>> It can support up to
>> - 2 IOB PMU instances
>> - 8 L3C PMU instances
>> - 2 MCB PMU instances
>> - 8 MCU PMU instances
>
> Please elaborate on the differences compared with prior versions.
>
> I see that counters are 64-bit. Are there any other important details to
> be aware of?
Yes, let me add 64 bit counter into the patch description. Beside of
that, I don't think anything else besides more PMU instances compared
with the previous version.
>
> [...]
>
>> +static struct attribute *l3c_pmu_v3_events_attrs[] = {
>
>> + XGENE_PMU_EVENT_ATTR(read-hit, 0x01),
>> + XGENE_PMU_EVENT_ATTR(read-miss, 0x02),
>
>> + XGENE_PMU_EVENT_ATTR(reads, 0x08),
>> + XGENE_PMU_EVENT_ATTR(writes,0x09),
>
>> +};
>
> Some of these are singular (e.g. "read-hit", "read-miss"), while others
> are plural (e.g. "reads", "writes").
>
> The existing driver use the singular form. Please remain consistent with
> that. e.g. turn "reads" into "read".
Will fix it.
>
>> + XGENE_PMU_EVENT_ATTR(PA-req-buf-alloc-all, 0x01),
>
> Likewise, please consistently use lower case.
Yes
>
>> + XGENE_PMU_EVENT_ATTR(pmu-act-sent, 0x01),
>
> Surely we don't need "pmu" in the event names?
Yes, will remove "pmu".
>
> [...]
>
>> static inline u64 xgene_pmu_read_counter32(struct xgene_pmu_dev *pmu_dev,
>> int idx)
>> {
>> return (u64)readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
>> }
>
> I don't think the cast is necessary. Please remove it.
>
>> static inline void
>> +xgene_pmu_write_counter64(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
>> +{
>> + u32 cnt_lo, cnt_hi;
>> +
>> + cnt_lo = val & 0x;
>> + cnt_hi = val >> 32;
>
> cnt_hi = upper_32_bits(val);
> cnt_lo = lower_32_bits(val);
>
> (both are in )
>
>> +
>> + /* v3 has 64-bit counter registers composed by 2 32-bit registers */
>> + xgene_pmu_write_counter32(pmu_dev, 2 * idx, val);
>> + xgene_pmu_write_counter32(pmu_dev, 2 * idx + 1, val >> 32);
>
> Please use cnt_hi and cnt_lo for the writes.
>
> [...]
>
>> @@ -621,7 +989,7 @@ static void xgene_perf_event_set_period(struct
>> perf_event *event)
>> struct xgene_pmu *xgene_pmu = pmu_dev->parent;
>> struct hw_perf_event *hw = &event->hw;
>> /*
>> - * The X-Gene PMU counters have a period of 2^32. To account for the
>> + * The X-Gene PMU counters have a period of 2^32 or more. To account
>> for the
>>* possiblity of extreme interrupt latency we program for a period of
>>* half that. Hopefully we can handle the interrupt before another 2^31
>>* events occur and the counter overtakes its previous value.
>
> This comment is now a little out-of-date, as we don't start 64-bit
> counters at half their period.
>
> I guess we don't expect a 64-bit counter to overflow, so can we state
> that in the comment?
How about this one.
/*
* For 32 bit counter, it has a period of 2^32. To account for the
* possibility of extreme interrupt latency we program for a period of
* half that. Hopefully, we can handle the interrupt before another 2^31
* events occur and the counter overtakes its previous value.
* For 64 bit counter, we don't expect it overflow.
*/
>
> [...]
>
>> -static int acpi_pmu_probe_active_mcb_mcu(struct xgene_pmu *xgene_pmu,
>> +static int acpi_pmu_probe_active_mcb_mcu_l3c(struct xgene_pmu *xgene_pmu,
>>struct platform_device *pdev)
>> {
>> void __iomem *csw_csr, *mcba_csr, *mcbb_csr;
>> struct resource *res;
>> unsigned int reg;
>> + u32 mcb0routing;
>> + u32 mcb1routing;
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> csw_csr = devm_ioremap_resource(&pdev->dev, res);
>> @@ -899,41 +1309,72 @@ static int acpi_pmu_probe_active_mcb_mcu(struct
>> xgene_pmu *xgene_pmu,
>> return PTR_ERR(csw_csr);
>> }
>>
>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>> - mcba_csr = devm_ioremap_resource(&pdev->dev, res);
>> - if (IS_ERR(mcba_csr)) {
>> - dev_err(&pdev->dev, "ioremap failed for MCBA CSR resource\n");
>> - return PTR_ERR(mcba_csr);
>> - }
>> -
>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
>> - mcbb_csr = devm_ioremap_resource(&pdev->dev, res);
>> - if (IS_ERR(mcbb_csr)) {
>> - dev_err(&pdev->dev, "ioremap failed for MCBB CSR resource\n");
>> - return PTR_ERR(mcbb_csr);
>> -
Re: [PATCH v2 1/3] perf: xgene: Parse PMU subnode from the match table
Hi Mark,
On Fri, Jun 2, 2017 at 10:23 AM, Mark Rutland wrote:
> On Fri, Jun 02, 2017 at 09:54:32AM -0700, Hoan Tran wrote:
>> On Fri, Jun 2, 2017 at 7:59 AM, Mark Rutland wrote:
>> > On Mon, Apr 03, 2017 at 09:47:55AM -0700, Hoan Tran wrote:
>> >> +static const struct acpi_device_id *xgene_pmu_acpi_match_type(
>> >> + const struct acpi_device_id *ids,
>> >> + struct acpi_device *adev)
>> >> +{
>> >> + const struct acpi_device_id *match_id = NULL;
>> >> + const struct acpi_device_id *id;
>> >> +
>> >> + for (id = ids; id->id[0] || id->cls; id++) {
>> >> + if (!acpi_match_device_ids(adev, id))
>> >> + match_id = id;
>> >> + else if (match_id)
>> >> + break;
>> >> + }
>> >> +
>> >> + return match_id;
>> >> +}
>> >
>> > I don't believe this look is necessary. AFAICT, acpi_match_device_ids()
>> > already iterates over the id table it is given.
>>
>> The acpi_match_device_ids() function just returns if a device ID is
>> available on the given list. It does not return the first matching ID.
>> That's the reason I created this function to find the first matching ID.
>
> Ah, I see. Thanks for correcting me!
>
> Can we use acpi_match_device(ids, &adev->dev), or is that the wrong dev?
They are subnode device, so they don't have full dev. Because of that,
acpi_match_device doesn't work.
Thanks
Hoan
>
> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 11/17] cgroup: Implement new thread mode semantics
On 06/01/2017 05:18 PM, Tejun Heo wrote: > Hello, > > On Thu, Jun 01, 2017 at 05:12:42PM -0400, Waiman Long wrote: >> Are you referring to keeping the no internal process restriction and >> document how to work around that instead? I would like to hear what >> workarounds are currently being used. > What we've been talking about all along - just creating explicit leaf > nodes. > >> Anyway, you currently allow internal process in thread mode, but not in >> non-thread mode. I would prefer no such restriction in both thread and >> non-thread mode. > Heh, so, these aren't arbitrary. The contraint is tied to > implementing resource domains and thread subtree doesn't have resource > domains in them, so they don't need the constraint. I'm sorry about > the short replies but I'm kinda really tied up right now. I'm gonna > do the thread mode so that it can be agnostic w.r.t. the internal > process constraint and I think it could be helpful to decouple these > discussions. We've been having this discussion for a couple years now > and it looks like we're gonna go through it all over, which is fine, > but let's at least keep that separate. I wouldn't argue further on that if you insist. However, I still want to relax the constraint somewhat by abandoning the no internal process constraint when only threaded controllers (non-resource domains) are enabled even when thread mode has not been explicitly enabled. It is a modified version my second alternative. Now the question is which controllers are considered to be resource domains. I think memory and blkio are in the list. What else do you think should be considered resource domains? Cheers, Longman any of the resource domains (!threaded) controllers are enabled. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 7/7] Documentation/dev-tools: Add kselftest_harness documentation
Hi Mickaël,
On 05/26/2017 12:44 PM, Mickaël Salaün wrote:
> Add ReST metadata to kselftest_harness.h to be able to include the
> comments in the Sphinx documentation.
>
These don't belong the change log. These types of changes are for
reviewers benefit and should be added between the sign-of block
and the start of diff.
I had to fix a few patches in this series before commit.
> Changes since v4:
> * exclude the TEST_API() changes (requested by Kees Cook)
>
> Changes since v3:
> * document macros as actual functions (suggested by Jonathan Corbet)
> * remove the TEST_API() wrapper to expose the underlying macro arguments
> to the documentation tools
> * move and cleanup comments
>
> Changes since v2:
> * add reference to the full documentation in the header file (suggested
> by Kees Cook)
>
> Signed-off-by: Mickaël Salaün
> Cc: Andy Lutomirski
> Cc: Jonathan Corbet
> Cc: Kees Cook
> Cc: Shuah Khan
> Cc: Will Drewry
git am isn't happy:
Applying: Documentation/dev-tools: Add kselftest_harness documentation
.git/rebase-apply/patch:47: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Please fix and resend.
-- Shuah
> ---
> Documentation/dev-tools/kselftest.rst | 34 +++
> tools/testing/selftests/kselftest_harness.h | 415
> ++--
> 2 files changed, 364 insertions(+), 85 deletions(-)
>
> diff --git a/Documentation/dev-tools/kselftest.rst
> b/Documentation/dev-tools/kselftest.rst
> index 9232ce94612c..a92fa181b6cf 100644
> --- a/Documentation/dev-tools/kselftest.rst
> +++ b/Documentation/dev-tools/kselftest.rst
> @@ -120,3 +120,37 @@ Contributing new tests (details)
> executable which is not tested by default.
> TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
> test.
> +
> +Test Harness
> +
> +
> +The kselftest_harness.h file contains useful helpers to build tests. The
> tests
> +from tools/testing/selftests/seccomp/seccomp_bpf.c can be used as example.
> +
> +Example
> +---
> +
> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
> +:doc: example
> +
> +
> +Helpers
> +---
> +
> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
> +:functions: TH_LOG TEST TEST_SIGNAL FIXTURE FIXTURE_DATA FIXTURE_SETUP
> +FIXTURE_TEARDOWN TEST_F TEST_HARNESS_MAIN
> +
> +Operators
> +-
> +
> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
> +:doc: operators
> +
> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
> +:functions: ASSERT_EQ ASSERT_NE ASSERT_LT ASSERT_LE ASSERT_GT ASSERT_GE
> +ASSERT_NULL ASSERT_TRUE ASSERT_NULL ASSERT_TRUE ASSERT_FALSE
> +ASSERT_STREQ ASSERT_STRNE EXPECT_EQ EXPECT_NE EXPECT_LT
> +EXPECT_LE EXPECT_GT EXPECT_GE EXPECT_NULL EXPECT_TRUE
> +EXPECT_FALSE EXPECT_STREQ EXPECT_STRNE
> +
> diff --git a/tools/testing/selftests/kselftest_harness.h
> b/tools/testing/selftests/kselftest_harness.h
> index 45f807ce37e1..c56f72e07cd7 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -4,41 +4,49 @@
> *
> * kselftest_harness.h: simple C unit test helper.
> *
> - * Usage:
> - * #include "../kselftest_harness.h"
> - * TEST(standalone_test) {
> - * do_some_stuff;
> - * EXPECT_GT(10, stuff) {
> - *stuff_state_t state;
> - *enumerate_stuff_state(&state);
> - *TH_LOG("expectation failed with state: %s", state.msg);
> - * }
> - * more_stuff;
> - * ASSERT_NE(some_stuff, NULL) TH_LOG("how did it happen?!");
> - * last_stuff;
> - * EXPECT_EQ(0, last_stuff);
> - * }
> - *
> - * FIXTURE(my_fixture) {
> - * mytype_t *data;
> - * int awesomeness_level;
> - * };
> - * FIXTURE_SETUP(my_fixture) {
> - * self->data = mytype_new();
> - * ASSERT_NE(NULL, self->data);
> - * }
> - * FIXTURE_TEARDOWN(my_fixture) {
> - * mytype_free(self->data);
> - * }
> - * TEST_F(my_fixture, data_is_good) {
> - * EXPECT_EQ(1, is_my_data_good(self->data));
> - * }
> - *
> - * TEST_HARNESS_MAIN
> + * See documentation in Documentation/dev-tools/kselftest.rst
> *
> * API inspired by code.google.com/p/googletest
> */
>
> +/**
> + * DOC: example
> + *
> + * .. code-block:: c
> + *
> + *#include "../kselftest_harness.h"
> + *
> + *TEST(standalone_test) {
> + * do_some_stuff;
> + * EXPECT_GT(10, stuff) {
> + * stuff_state_t state;
> + * enumerate_stuff_state(&state);
> + * TH_LOG("expectation failed with state: %s", state.msg);
> + * }
> + * more_stuff;
> + * ASSERT_NE(some_stuff, NULL) TH_LOG("how did it happen?!");
> + * last_stuff;
> + * EXPECT_EQ(0, last_stuff);
> + *}
> + *
> + *FIXTURE(my_fixture) {
> + * mytype_t *data;
> + * int awesomeness_level;
> + *};
> + *FIXTURE_SETUP(my_fixture) {
> + * self->data = mytype_new(
Re: [PATCH v5 3/7] selftests/seccomp: Force rebuild according to dependencies
Hi Mickaël, On 05/26/2017 12:43 PM, Mickaël Salaün wrote: > Rebuild the seccomp tests when kselftest_harness.h is updated. > > Signed-off-by: Mickaël Salaün > Acked-by: Kees Cook > Cc: Andy Lutomirski > Cc: Shuah Khan > Cc: Will Drewry > --- > tools/testing/selftests/seccomp/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/seccomp/Makefile > b/tools/testing/selftests/seccomp/Makefile > index 5fa6fd2246b1..aeb0c805f3ca 100644 > --- a/tools/testing/selftests/seccomp/Makefile > +++ b/tools/testing/selftests/seccomp/Makefile > @@ -4,3 +4,5 @@ LDFLAGS += -lpthread > > include ../lib.mk > > +$(TEST_GEN_PROGS): seccomp_bpf.c ../kselftest_harness.h > + $(CC) $(CFLAGS) $(LDFLAGS) $< -o $@ > This change breaks seccomp build: make -C tools/testing/selftests/seccomp/ make: Entering directory '/mnt/data/lkml/linux_4.12/tools/testing/selftests/seccomp' make: *** No rule to make target '../kselftest_harness.h', needed by '/mnt/data/lkml/linux_4.12/tools/testing/selftests/seccomp/seccomp_bpf'. Stop. make: Leaving directory '/mnt/data/lkml/linux_4.12/tools/testing/selftests/seccomp' shuah@mazurka:/mnt/data/lkml/linux_4.12$ cd tools/testing/selftests/seccomp/ shuah@mazurka:/mnt/data/lkml/linux_4.12/tools/testing/selftests/seccomp$ make make: *** No rule to make target '../kselftest_harness.h', needed by '/mnt/data/lkml/linux_4.12/tools/testing/selftests/seccomp/seccomp_bpf'. Stop. Did you happen to try building with this change? thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] perf: xgene: Parse PMU subnode from the match table
On Fri, Jun 02, 2017 at 09:54:32AM -0700, Hoan Tran wrote:
> On Fri, Jun 2, 2017 at 7:59 AM, Mark Rutland wrote:
> > On Mon, Apr 03, 2017 at 09:47:55AM -0700, Hoan Tran wrote:
> >> +static const struct acpi_device_id *xgene_pmu_acpi_match_type(
> >> + const struct acpi_device_id *ids,
> >> + struct acpi_device *adev)
> >> +{
> >> + const struct acpi_device_id *match_id = NULL;
> >> + const struct acpi_device_id *id;
> >> +
> >> + for (id = ids; id->id[0] || id->cls; id++) {
> >> + if (!acpi_match_device_ids(adev, id))
> >> + match_id = id;
> >> + else if (match_id)
> >> + break;
> >> + }
> >> +
> >> + return match_id;
> >> +}
> >
> > I don't believe this look is necessary. AFAICT, acpi_match_device_ids()
> > already iterates over the id table it is given.
>
> The acpi_match_device_ids() function just returns if a device ID is
> available on the given list. It does not return the first matching ID.
> That's the reason I created this function to find the first matching ID.
Ah, I see. Thanks for correcting me!
Can we use acpi_match_device(ids, &adev->dev), or is that the wrong dev?
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] perf: xgene: Parse PMU subnode from the match table
Hi Mark
On Fri, Jun 2, 2017 at 7:59 AM, Mark Rutland wrote:
> Hi Hoan,
>
> Apologies for the last reply.
>
> On Mon, Apr 03, 2017 at 09:47:55AM -0700, Hoan Tran wrote:
>> +static const struct acpi_device_id xgene_pmu_acpi_type_match[] = {
>> + {"APMC0D5D", PMU_TYPE_L3C},
>> + {"APMC0D5E", PMU_TYPE_IOB},
>> + {"APMC0D5F", PMU_TYPE_MCB},
>> + {"APMC0D60", PMU_TYPE_MC},
>> + {},
>> +};
>> +
>> +static const struct acpi_device_id *xgene_pmu_acpi_match_type(
>> + const struct acpi_device_id *ids,
>> + struct acpi_device *adev)
>> +{
>> + const struct acpi_device_id *match_id = NULL;
>> + const struct acpi_device_id *id;
>> +
>> + for (id = ids; id->id[0] || id->cls; id++) {
>> + if (!acpi_match_device_ids(adev, id))
>> + match_id = id;
>> + else if (match_id)
>> + break;
>> + }
>> +
>> + return match_id;
>> +}
>
> I don't believe this look is necessary. AFAICT, acpi_match_device_ids()
> already iterates over the id table it is given.
The acpi_match_device_ids() function just returns if a device ID is
available on the given list. It does not return the first matching ID.
That's the reason I created this function to find the first matching ID.
>
>> +
>> static acpi_status acpi_pmu_dev_add(acpi_handle handle, u32 level,
>> void *data, void **return_value)
>> {
>> + const struct acpi_device_id *acpi_id;
>> struct xgene_pmu *xgene_pmu = data;
>> struct xgene_pmu_dev_ctx *ctx;
>> struct acpi_device *adev;
>> @@ -1059,17 +1085,11 @@ static acpi_status acpi_pmu_dev_add(acpi_handle
>> handle, u32 level,
>> if (acpi_bus_get_status(adev) || !adev->status.present)
>> return AE_OK;
>>
>> - if (!strcmp(acpi_device_hid(adev), "APMC0D5D"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_L3C);
>> - else if (!strcmp(acpi_device_hid(adev), "APMC0D5E"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_IOB);
>> - else if (!strcmp(acpi_device_hid(adev), "APMC0D5F"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MCB);
>> - else if (!strcmp(acpi_device_hid(adev), "APMC0D60"))
>> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MC);
>> - else
>> - ctx = NULL;
>> + acpi_id = xgene_pmu_acpi_match_type(xgene_pmu_acpi_type_match, adev);
>> + if (!acpi_id)
>> + return AE_OK;
>
> As above, and as I covered in my reply to v1, I think the above should
> be:
>
> acpi_id = acpi_match_device_ids(adev, xgene_pmu_acpi_type_match);
> if (!acpi_id)
> return AE_OK;
>
> ... or am I missing something?
The same above.
Thanks
Hoan
>
> With that change:
>
> Acked-by: Mark Rutland
>
> Thanks,
> Mark.
>
>>
>> + ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, (u32)acpi_id->driver_data);
>> if (!ctx)
>> return AE_OK;
>>
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] perf: xgene: Add support for SoC PMU version 3
Hi Hoan,
Apologies for the delay in getting to this.
On Mon, Apr 03, 2017 at 09:47:57AM -0700, Hoan Tran wrote:
> This patch adds support for SoC-wide (AKA uncore) Performance Monitoring
> Unit version 3.
>
> It can support up to
> - 2 IOB PMU instances
> - 8 L3C PMU instances
> - 2 MCB PMU instances
> - 8 MCU PMU instances
Please elaborate on the differences compared with prior versions.
I see that counters are 64-bit. Are there any other important details to
be aware of?
[...]
> +static struct attribute *l3c_pmu_v3_events_attrs[] = {
> + XGENE_PMU_EVENT_ATTR(read-hit, 0x01),
> + XGENE_PMU_EVENT_ATTR(read-miss, 0x02),
> + XGENE_PMU_EVENT_ATTR(reads, 0x08),
> + XGENE_PMU_EVENT_ATTR(writes,0x09),
> +};
Some of these are singular (e.g. "read-hit", "read-miss"), while others
are plural (e.g. "reads", "writes").
The existing driver use the singular form. Please remain consistent with
that. e.g. turn "reads" into "read".
> + XGENE_PMU_EVENT_ATTR(PA-req-buf-alloc-all, 0x01),
Likewise, please consistently use lower case.
> + XGENE_PMU_EVENT_ATTR(pmu-act-sent, 0x01),
Surely we don't need "pmu" in the event names?
[...]
> static inline u64 xgene_pmu_read_counter32(struct xgene_pmu_dev *pmu_dev,
> int idx)
> {
> return (u64)readl(pmu_dev->inf->csr + PMU_PMEVCNTR0 + (4 * idx));
> }
I don't think the cast is necessary. Please remove it.
> static inline void
> +xgene_pmu_write_counter64(struct xgene_pmu_dev *pmu_dev, int idx, u64 val)
> +{
> + u32 cnt_lo, cnt_hi;
> +
> + cnt_lo = val & 0x;
> + cnt_hi = val >> 32;
cnt_hi = upper_32_bits(val);
cnt_lo = lower_32_bits(val);
(both are in )
> +
> + /* v3 has 64-bit counter registers composed by 2 32-bit registers */
> + xgene_pmu_write_counter32(pmu_dev, 2 * idx, val);
> + xgene_pmu_write_counter32(pmu_dev, 2 * idx + 1, val >> 32);
Please use cnt_hi and cnt_lo for the writes.
[...]
> @@ -621,7 +989,7 @@ static void xgene_perf_event_set_period(struct perf_event
> *event)
> struct xgene_pmu *xgene_pmu = pmu_dev->parent;
> struct hw_perf_event *hw = &event->hw;
> /*
> - * The X-Gene PMU counters have a period of 2^32. To account for the
> + * The X-Gene PMU counters have a period of 2^32 or more. To account
> for the
>* possiblity of extreme interrupt latency we program for a period of
>* half that. Hopefully we can handle the interrupt before another 2^31
>* events occur and the counter overtakes its previous value.
This comment is now a little out-of-date, as we don't start 64-bit
counters at half their period.
I guess we don't expect a 64-bit counter to overflow, so can we state
that in the comment?
[...]
> -static int acpi_pmu_probe_active_mcb_mcu(struct xgene_pmu *xgene_pmu,
> +static int acpi_pmu_probe_active_mcb_mcu_l3c(struct xgene_pmu *xgene_pmu,
>struct platform_device *pdev)
> {
> void __iomem *csw_csr, *mcba_csr, *mcbb_csr;
> struct resource *res;
> unsigned int reg;
> + u32 mcb0routing;
> + u32 mcb1routing;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> csw_csr = devm_ioremap_resource(&pdev->dev, res);
> @@ -899,41 +1309,72 @@ static int acpi_pmu_probe_active_mcb_mcu(struct
> xgene_pmu *xgene_pmu,
> return PTR_ERR(csw_csr);
> }
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> - mcba_csr = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(mcba_csr)) {
> - dev_err(&pdev->dev, "ioremap failed for MCBA CSR resource\n");
> - return PTR_ERR(mcba_csr);
> - }
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> - mcbb_csr = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(mcbb_csr)) {
> - dev_err(&pdev->dev, "ioremap failed for MCBB CSR resource\n");
> - return PTR_ERR(mcbb_csr);
> - }
> -
> reg = readl(csw_csr + CSW_CSWCR);
> - if (reg & CSW_CSWCR_DUALMCB_MASK) {
> - /* Dual MCB active */
> - xgene_pmu->mcb_active_mask = 0x3;
> - /* Probe all active MC(s) */
> - reg = readl(mcbb_csr + CSW_CSWCR);
> - xgene_pmu->mc_active_mask =
> - (reg & MCBADDRMR_DUALMCU_MODE_MASK) ? 0xF : 0x5;
> + if (xgene_pmu->version == PCP_PMU_V3) {
> + mcb0routing = CSW_CSWCR_MCB0_ROUTING(reg);
> + mcb1routing = CSW_CSWCR_MCB0_ROUTING(reg);
> + if (reg & CSW_CSWCR_DUALMCB_MASK) {
> + /* Dual MCB active */
> + xgene_pmu->mcb_active_mask = 0x3;
> + /* Probe all active L3C(s), maximum is 8 */
> + xgene_pmu->l3c_active_mask
Re: [RFC PATCH] mm, oom: cgroup-aware OOM-killer
On Fri, Jun 02, 2017 at 10:43:33AM +0200, Michal Hocko wrote: > On Wed 31-05-17 14:01:45, Johannes Weiner wrote: > > On Wed, May 31, 2017 at 06:25:04PM +0200, Michal Hocko wrote: > > > > > + /* > > > > >* If current has a pending SIGKILL or is exiting, then > > > > > automatically > > > > >* select it. The goal is to allow it to allocate so that it > > > > > may > > > > >* quickly exit and free its memory. > > > > > > > > > > Please note that I haven't explored how much of the infrastructure > > > > > needed for the OOM decision making is available to modules. But we can > > > > > export a lot of what we currently have in oom_kill.c. I admit it might > > > > > turn out that this is simply not feasible but I would like this to be > > > > > at > > > > > least explored before we go and implement yet another hardcoded way to > > > > > handle (see how I didn't use policy ;)) OOM situation. > > > > > > > > ;) > > > > > > > > My doubt here is mainly that we'll see many (or any) real-life cases > > > > materialize that cannot be handled with cgroups and scoring. These are > > > > powerful building blocks on which userspace can implement all kinds of > > > > policy and sorting algorithms. > > > > > > > > So this seems like a lot of churn and complicated code to handle one > > > > extension. An extension that implements basic functionality. > > > > > > Well, as I've said I didn't get to explore this path so I have only a > > > very vague idea what we would have to export to implement e.g. the > > > proposed oom killing strategy suggested in this thread. Unfortunatelly I > > > do not have much time for that. I do not want to block a useful work > > > which you have a usecase for but I would be really happy if we could > > > consider longer term plans before diving into a "hardcoded" > > > implementation. We didn't do that previously and we are left with > > > oom_kill_allocating_task and similar one off things. > > > > As I understand it, killing the allocating task was simply the default > > before the OOM killer and was added as a compat knob. I really doubt > > anybody is using it at this point, and we could probably delete it. > > I might misremember but my recollection is that SGI simply had too > large machines with too many processes and so the task selection was > very expensinve. Cgroup-aware OOM killer can be much better in case of large number of processes, as we don't have to iterate over all processes locking each mm, and can select an appropriate cgroup based mostly on lockless counters. Of course, it depends on concrete setup, but it can be much more efficient under right circumstances. > > > I appreciate your concern of being too short-sighted here, but the > > fact that I cannot point to more usecases isn't for lack of trying. I > > simply don't see the endless possibilities of usecases that you do. > > > > It's unlikely for more types of memory domains to pop up besides MMs > > and cgroups. (I mentioned vmas, but that just seems esoteric. And we > > have panic_on_oom for whole-system death. What else could there be?) > > > > And as I pointed out, there is no real evidence that the current > > system for configuring preferences isn't sufficient in practice. > > > > That's my thoughts on exploring. I'm not sure what else to do before > > it feels like running off into fairly contrived hypotheticals. > > Yes, I do not want hypotheticals to block an otherwise useful feature, > of course. But I haven't heard a strong argument why a module based > approach would be a more maintenance burden longterm. From a very quick > glance over patches Roman has posted yesterday it seems that a large > part of the existing oom infrastructure can be reused reasonably. I have nothing against module based approach, but I don't think that a module should implement anything rather than then oom score calculation (for a process and a cgroup). Maybe only some custom method for killing, but I can't really imagine anything reasonable except killing one "worst" process or killing whole cgroup(s). In case of a system wide OOM, we have to free some memory quickly, and this means we can't do anything much more complex, than killing some process(es). So, in my understanding, what you're suggesting is not against the proposed approach at all. We still need to iterate over cgroups, somehow define their badness, find the worst one and destroy it. In my v2 I've tried to separate these two potentially customizable areas in two simple functions: mem_cgroup_oom_badness() and mem_cgroup_kill_oom_victim(). So we can add an ability to customize these functions (and similar stuff for processes), if we'll have some real examples of where the proposed functionality is insufficient. Do you have any examples which can't be covered by this approach? Thanks! Roman -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More major
Re: [PATCH v2 1/3] perf: xgene: Parse PMU subnode from the match table
Hi Hoan,
Apologies for the last reply.
On Mon, Apr 03, 2017 at 09:47:55AM -0700, Hoan Tran wrote:
> +static const struct acpi_device_id xgene_pmu_acpi_type_match[] = {
> + {"APMC0D5D", PMU_TYPE_L3C},
> + {"APMC0D5E", PMU_TYPE_IOB},
> + {"APMC0D5F", PMU_TYPE_MCB},
> + {"APMC0D60", PMU_TYPE_MC},
> + {},
> +};
> +
> +static const struct acpi_device_id *xgene_pmu_acpi_match_type(
> + const struct acpi_device_id *ids,
> + struct acpi_device *adev)
> +{
> + const struct acpi_device_id *match_id = NULL;
> + const struct acpi_device_id *id;
> +
> + for (id = ids; id->id[0] || id->cls; id++) {
> + if (!acpi_match_device_ids(adev, id))
> + match_id = id;
> + else if (match_id)
> + break;
> + }
> +
> + return match_id;
> +}
I don't believe this look is necessary. AFAICT, acpi_match_device_ids()
already iterates over the id table it is given.
> +
> static acpi_status acpi_pmu_dev_add(acpi_handle handle, u32 level,
> void *data, void **return_value)
> {
> + const struct acpi_device_id *acpi_id;
> struct xgene_pmu *xgene_pmu = data;
> struct xgene_pmu_dev_ctx *ctx;
> struct acpi_device *adev;
> @@ -1059,17 +1085,11 @@ static acpi_status acpi_pmu_dev_add(acpi_handle
> handle, u32 level,
> if (acpi_bus_get_status(adev) || !adev->status.present)
> return AE_OK;
>
> - if (!strcmp(acpi_device_hid(adev), "APMC0D5D"))
> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_L3C);
> - else if (!strcmp(acpi_device_hid(adev), "APMC0D5E"))
> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_IOB);
> - else if (!strcmp(acpi_device_hid(adev), "APMC0D5F"))
> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MCB);
> - else if (!strcmp(acpi_device_hid(adev), "APMC0D60"))
> - ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, PMU_TYPE_MC);
> - else
> - ctx = NULL;
> + acpi_id = xgene_pmu_acpi_match_type(xgene_pmu_acpi_type_match, adev);
> + if (!acpi_id)
> + return AE_OK;
As above, and as I covered in my reply to v1, I think the above should
be:
acpi_id = acpi_match_device_ids(adev, xgene_pmu_acpi_type_match);
if (!acpi_id)
return AE_OK;
... or am I missing something?
With that change:
Acked-by: Mark Rutland
Thanks,
Mark.
>
> + ctx = acpi_get_pmu_hw_inf(xgene_pmu, adev, (u32)acpi_id->driver_data);
> if (!ctx)
> return AE_OK;
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] usb: gadget: f_uac2: split out audio core
On Tue, May 30, 2017 at 5:13 AM, Ruslan Bilovol
wrote:
> On Mon, May 22, 2017 at 6:58 PM, Jassi Brar wrote:
>> On Thu, May 18, 2017 at 4:07 AM, Ruslan Bilovol
>> wrote:
>>> Abstract the peripheral side ALSA sound card code from
>>> the f_uac2 function into a component that can be called
>>> by various functions, so the various flavors can be split
>>> apart and selectively reused.
>>>
>>> Visible changes:
>>> - add uac_params structure to pass audio paramteres for
>>>g_audio_setup
>>> - make ALSA sound card's name configurable
>>> - add [in/out]_ep_maxpsize
>>> - allocate snd_uac_chip structure during g_audio_setup
>>> - add u_audio_[start/stop]_[capture/playback] functions
>>>
>>> Signed-off-by: Ruslan Bilovol
>>> ---
>>> drivers/usb/gadget/Kconfig| 4 +
>>> drivers/usb/gadget/function/Makefile | 1 +
>>> drivers/usb/gadget/function/f_uac2.c | 721
>>> --
>>> drivers/usb/gadget/function/u_audio.c | 661 +++
>>> drivers/usb/gadget/function/u_audio.h | 95 +
>>> drivers/usb/gadget/legacy/Kconfig | 1 +
>>> 6 files changed, 846 insertions(+), 637 deletions(-)
>>> create mode 100644 drivers/usb/gadget/function/u_audio.c
>>> create mode 100644 drivers/usb/gadget/function/u_audio.h
>>>
>>> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
>>> index c164d6b..2ba0ace 100644
>>> --- a/drivers/usb/gadget/Kconfig
>>> +++ b/drivers/usb/gadget/Kconfig
>>> @@ -158,6 +158,9 @@ config USB_U_SERIAL
>>> config USB_U_ETHER
>>> tristate
>>>
>>> +config USB_U_AUDIO
>>> + tristate
>>> +
>>> config USB_F_SERIAL
>>> tristate
>>>
>>> @@ -381,6 +384,7 @@ config USB_CONFIGFS_F_UAC2
>>> depends on SND
>>> select USB_LIBCOMPOSITE
>>> select SND_PCM
>>> + select USB_U_AUDIO
>>> select USB_F_UAC2
>>> help
>>> This Audio function is compatible with USB Audio Class
>>> diff --git a/drivers/usb/gadget/function/Makefile
>>> b/drivers/usb/gadget/function/Makefile
>>> index cb8c225..b29f2ae 100644
>>> --- a/drivers/usb/gadget/function/Makefile
>>> +++ b/drivers/usb/gadget/function/Makefile
>>> @@ -32,6 +32,7 @@ usb_f_mass_storage-y := f_mass_storage.o
>>> storage_common.o
>>> obj-$(CONFIG_USB_F_MASS_STORAGE)+= usb_f_mass_storage.o
>>> usb_f_fs-y := f_fs.o
>>> obj-$(CONFIG_USB_F_FS) += usb_f_fs.o
>>> +obj-$(CONFIG_USB_U_AUDIO) += u_audio.o
>>> usb_f_uac1-y := f_uac1.o u_uac1.o
>>> obj-$(CONFIG_USB_F_UAC1) += usb_f_uac1.o
>>> usb_f_uac2-y := f_uac2.o
>>> diff --git a/drivers/usb/gadget/function/f_uac2.c
>>> b/drivers/usb/gadget/function/f_uac2.c
>>> index d4565b5..059a14a 100644
>>> --- a/drivers/usb/gadget/function/f_uac2.c
>>> +++ b/drivers/usb/gadget/function/f_uac2.c
>>> @@ -15,10 +15,7 @@
>>> #include
>>> #include
>>>
>>> -#include
>>> -#include
>>> -#include
>>> -
>>> +#include "u_audio.h"
>>> #include "u_uac2.h"
>>>
>>> /*
>>> @@ -50,455 +47,23 @@
>>> #define UNFLW_CTRL 8
>>> #define OVFLW_CTRL 10
>>>
>>> -struct uac2_req {
>>> - struct uac2_rtd_params *pp; /* parent param */
>>> - struct usb_request *req;
>>> -};
>>> -
>>> -struct uac2_rtd_params {
>>> - struct snd_uac2_chip *uac2; /* parent chip */
>>> - bool ep_enabled; /* if the ep is enabled */
>>> - /* Size of the ring buffer */
>>> - size_t dma_bytes;
>>> - unsigned char *dma_area;
>>> -
>>> - struct snd_pcm_substream *ss;
>>> -
>>> - /* Ring buffer */
>>> - ssize_t hw_ptr;
>>> -
>>> - void *rbuf;
>>> -
>>> - size_t period_size;
>>> -
>>> - unsigned max_psize;
>>> - struct uac2_req *ureq;
>>> -
>>> - spinlock_t lock;
>>> -};
>>> -
>>> -struct snd_uac2_chip {
>>> - struct uac2_rtd_params p_prm;
>>> - struct uac2_rtd_params c_prm;
>>> -
>>> - struct snd_card *card;
>>> - struct snd_pcm *pcm;
>>> -
>>> - /* timekeeping for the playback endpoint */
>>> - unsigned int p_interval;
>>> - unsigned int p_residue;
>>> -
>>> - /* pre-calculated values for playback iso completion */
>>> - unsigned int p_pktsize;
>>> - unsigned int p_pktsize_residue;
>>> - unsigned int p_framesize;
>>> +struct f_uac2 {
>>> + struct g_audio g_audio;
>>> + u8 ac_intf, as_in_intf, as_out_intf;
>>> + u8 ac_alt, as_in_alt, as_out_alt; /* needed for get_alt() */
>>> };
>>>
>>> -#define BUFF_SIZE_MAX (PAGE_SIZE * 16)
>>> -#define PRD_SIZE_MAX PAGE_SIZE
>>> -#define MIN_PERIODS4
>>> -
>>> -static struct snd_pcm_hardware uac2_pcm_hardware = {
>>> - .info = SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER
>>> -| SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID
>>> -| SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME,
>>> - .rates = SNDRV_PCM_RATE_CONTINUOUS,
>>> - .perio
Re: [PATCH v5 00/17] fs: introduce new writeback error reporting and convert ext2 and ext4 to use it
On Thu, 2017-06-01 at 23:25 -0600, Ross Zwisler wrote: > On Wed, May 31, 2017 at 08:45:23AM -0400, Jeff Layton wrote: > > v5: don't retrofit old API over the new infrastructure > > add fstype flag to indicate how wb errors are tracked within that fs > > add more function variants that take a errseq_t "since" value > > add second errseq_t to struct file to track metadata wb errors > > convert ext4 and ext2 to use the new APIs > > > > v4: several more cleanup patches > > documentation and kerneldoc comment updates > > fix bugs in gfs2 patches > > make sync_file_range use same error reporting semantics > > bugfixes in buffer.c > > convert nfs to new scheme (maybe bogus, can be dropped) > > > > v3: wb_err_t -> errseq_t conversion > > clean up places that re-set errors after calling filemap_* functions > > > > v2: introduce wb_err_t, use atomics > > > > This is v5 of the patchset to improve how we're tracking and reporting > > errors that occur during pagecache writeback. The main difference in > > this set from the last one is that I've stopped trying to retrofit the > > old error tracking API on top of the new one. This is more work since > > we'll have to touch each fs individually, but should be safer as the > > "since" values used for checking errors will be more deliberate. > > > > There are several situations where the kernel can "lose" errors that > > occur during writeback, such that fsync will return success even > > though it failed to write back some data previously. The basic idea > > here is to have the kernel be more deliberate about the point from > > which errors are checked to ensure that that doesn't happen. > > > > An additional aim of this set is to change the behavior of fsync in > > Linux to report writeback errors on all fds instead of just the first > > one. This allows writers to reliably tell whether their data made it to > > the backing device without having to coordinate fsync calls with other > > writers. > > > > To do this, we add a new typedef: errseq_t. This is a 32-bit value > > that can store an error code, and a sequence number so we can tell > > whether it has changed since we last sampled it. This allows us to > > record errors in the address_space and then report those errors only > > once per file description. > > > > This set just alters block device files, ext4 and the legacy ext2 > > driver. If this general approach seems acceptable, then I'll start > > converting other filesystems in follow-on patchsets. I'd also like > > to get this into linux-next as soon as possible to ensure that we're > > banging out any bugs that might be lurking here. > > > > I also have a couple of xfstests for this as well that I'll re-post > > soon. > > Can you tell me a baseline that this applies cleanly to, or give me a link to > a tree with these patches already applied? I've tried applying it to v4.11, > linux/master and mmots/master, and so far nothing has worked. It's basically on top of v4.12-rc3, but it may not apply cleanly without the pile of individual patches that I sent recently. It may be best to just pull down the "wberr" branch from my tree here: git://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git I was originally sending the prep patches as part of this series, but maintainers weren't picking them up, so I moved to sending them individually and then sending this pile as its own set. Many thanks for giving this a look and testing it! -- Jeff Layton -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] USB Audio Gadget refactoring
Hi, Ruslan Bilovol writes: > I came to this patch series when wanted to do two things: > - use UAC1 as virtual ALSA sound card on gadget side, >just like UAC2 is used so it's possible to do rate >resampling > - have both playback/capture support in UAC1 > > Since I wanted to have same behavior for both UAC1/UAC2, > obviously I've got an utility part (u_audio.c) for > virtual ALSA sound card handling like we have > for ethernet(u_ether) or serial(u_serial) functions. > Function-specific parts (f_uac1/f_uac2) became almost > as storage for class-specific USB descriptors, some > boilerplate for configfs, binding and few USB > config request handling. > > Originally in RFC [1] I've posted before, there was > major change to f_uac1 after that it couldn't do > direct play to existing ALSA sound card anymore, > representing audio on gadget side as virtual > ALSA sound card where audio streams are simply > sinked to and sourced from it, so it may break > current usecase for some people (and that's why > it was RFC). > > During RFC discussion, it was agreed to not touch > existing f_uac1 implementation and create new one > instead. This patchset (v4) introduced new function > named f_uac1_acard and doesn't touch current f_uac1 > implementation, so people still can use old behavior Do you have a pointer to the original RFC discussion where this was discussed? If we really *must* keep the old implementation, I would rather rename that to f_uac1_legacy. Still, I find it unlikely that anybody will care about the old implementation. > Now, it's possible to use existing user-space > applications for audio routing between Audio Gadget > and real sound card. I personally use alsaloop tool > from alsautils and have ability to create PCM > loopback between two different ALSA cards using > rate resampling, which was not possible with previous > "direct play to ALSA card" approach in f_uac1. this is really good result and will actually make it a lot easier for testing things out. > While here, also dropped redundant platform > driver/device creation in f_uac2 driver (as well as > didn't add "never implemented" volume/mute functionality > in f_uac1 to f_uac1_acard) that made this work even > easier to do. > > This series is tested with both legacy g_audio.ko and > modern configfs approaches under Ubuntu 14.04 (UAC1 and > UAC2) and under Windows7 x64 (UAC1 only) having > perfect results in all cases. > > Comments, testing are welcome. > > v4 changes: > - renamed f_uac1_newapi to f_uac1_acard that is >more meaningful I really don't get why you wanna keep both f_uac1 and f_uac1_acard. Why do we need to maintain the old uac1 implementation? Why two separate files? -- balbi signature.asc Description: PGP signature
Re: [PATCH v4 2/3] usb: gadget: f_uac2: split out audio core
Hi, Ruslan Bilovol writes: > Abstract the peripheral side ALSA sound card code from > the f_uac2 function into a component that can be called > by various functions, so the various flavors can be split > apart and selectively reused. > > Visible changes: > - add uac_params structure to pass audio paramteres for >g_audio_setup > - make ALSA sound card's name configurable > - add [in/out]_ep_maxpsize > - allocate snd_uac_chip structure during g_audio_setup > - add u_audio_[start/stop]_[capture/playback] functions > > Signed-off-by: Ruslan Bilovol this doesn't apply on testing/next, care to rebase? -- balbi signature.asc Description: PGP signature
Re: [RFC PATCH] mm, oom: cgroup-aware OOM-killer
On Wed 31-05-17 14:01:45, Johannes Weiner wrote:
> On Wed, May 31, 2017 at 06:25:04PM +0200, Michal Hocko wrote:
> > On Thu 25-05-17 13:08:05, Johannes Weiner wrote:
> > > Everything the user would want to dynamically program in the kernel,
> > > say with bpf, they could do in userspace and then update the scores
> > > for each group and task periodically.
> >
> > I am rather skeptical about dynamic scores. oom_{score_}adj has turned
> > to mere oom disable/enable knobs from my experience.
>
> That doesn't necessarily have to be a deficiency with the scoring
> system. I suspect that most people simply don't care as long as the
> the picks for OOM victims aren't entirely stupid.
>
> For example, we have a lot of machines that run one class of job. If
> we run OOM there isn't much preference we'd need to express; just kill
> one job - the biggest, whatever - and move on. (The biggest makes
> sense because if all jobs are basically equal it's as good as any
> other victim, but if one has a runaway bug it goes for that.)
>
> Where we have more than one job class, it actually is mostly one hipri
> and one lopri, in which case setting a hard limit on the lopri or the
> -1000 OOM score trick is enough.
>
> How many systems run more than two clearly distinguishable classes of
> workloads concurrently?
What about those which run different containers on a large physical
machine?
> I'm sure they exist. I'm just saying it doesn't surprise me that
> elaborate OOM scoring isn't all that wide-spread.
>
> > > The only limitation is that you have to recalculate and update the
> > > scoring tree every once in a while, whereas a bpf program could
> > > evaluate things just-in-time. But for that to matter in practice, OOM
> > > kills would have to be a fairly hot path.
> >
> > I am not really sure how to reliably implement "kill the memcg with the
> > largest process" strategy. And who knows how many others strategies will
> > pop out.
>
> That seems fairly contrived.
>
> What does it mean to divide memory into subdomains, but when you run
> out of physical memory you kill based on biggest task?
Well, the biggest task might be the runaway one and so killing it first
before you kill other innocent ones makes some sense to me.
> Sure, it frees memory and gets the system going again, so it's as good
> as any answer to overcommit gone wrong, I guess. But is that something
> you'd intentionally want to express from a userspace perspective?
>
[...]
> > > > Maybe. But that requires somebody to tweak the scoring which can be hard
> > > > from trivial.
> > >
> > > Why is sorting and picking in userspace harder than sorting and
> > > picking in the kernel?
> >
> > Because the userspace score based approach would be much more racy
> > especially in the busy system. This could lead to unexpected behavior
> > when OOM killer would kill a different than a run-away memcgs.
>
> How would it be easier to weigh priority against runaway detection
> inside the kernel?
You have better chances to catch such a process at the time of the OOM
because you do the check at the time of the OOM rather than sometimes
back in time when your monitor was able to run and check all the
existing processes (which alone can be rather time consuming so you do
not want to do that very often).
> > > > + /*
> > > > * If current has a pending SIGKILL or is exiting, then
> > > > automatically
> > > > * select it. The goal is to allow it to allocate so that it
> > > > may
> > > > * quickly exit and free its memory.
> > > >
> > > > Please note that I haven't explored how much of the infrastructure
> > > > needed for the OOM decision making is available to modules. But we can
> > > > export a lot of what we currently have in oom_kill.c. I admit it might
> > > > turn out that this is simply not feasible but I would like this to be at
> > > > least explored before we go and implement yet another hardcoded way to
> > > > handle (see how I didn't use policy ;)) OOM situation.
> > >
> > > ;)
> > >
> > > My doubt here is mainly that we'll see many (or any) real-life cases
> > > materialize that cannot be handled with cgroups and scoring. These are
> > > powerful building blocks on which userspace can implement all kinds of
> > > policy and sorting algorithms.
> > >
> > > So this seems like a lot of churn and complicated code to handle one
> > > extension. An extension that implements basic functionality.
> >
> > Well, as I've said I didn't get to explore this path so I have only a
> > very vague idea what we would have to export to implement e.g. the
> > proposed oom killing strategy suggested in this thread. Unfortunatelly I
> > do not have much time for that. I do not want to block a useful work
> > which you have a usecase for but I would be really happy if we could
> > consider longer term plans before diving into a "hardcoded"
> > implementation. We didn't do that previously and we are left with
> > oom_kill_alloca
